fix(files): prevent path traversal in /api/files endpoint

[sebastiondev]

Summary

The GET /api/files/{file_path:path} endpoint in api/routers/files.py is vulnerable to path traversal (CWE-22). An unauthenticated remote attacker can read arbitrary files that the server process can access (e.g. /etc/passwd, application config with API keys, private SSH keys) by supplying .. segments in the path parameter.

Vulnerability details

• File: api/routers/files.py
• Route: @router.get("/{file_path:path}") → mounted at /api/files/... via app.include_router(files_router, prefix=api_config.api_prefix) in api/app.py
• Auth: None. The router declares no dependencies=, and no app-wide auth middleware is installed. The server binds 0.0.0.0 by default.
• CWE: CWE-22 (Improper Limitation of a Pathname to a Restricted Directory)

Root cause

The previous containment check compared the raw, unresolved path against the allow-listed prefixes as a string:

abs_path = Path.cwd() / full_path
...
rel_path = abs_path.relative_to(Path.cwd())
rel_path_str = str(rel_path)
is_allowed = any(rel_path_str.startswith(prefix.rstrip('/')) for prefix in allowed_prefixes)

For input output/../../../etc/passwd:

1. full_path starts with output/, so the first allow-list check passes.
2. abs_path = Path.cwd() / "output/../../../etc/passwd" — pathlib does not normalize .. segments.
3. abs_path.relative_to(Path.cwd()) returns PosixPath('output/../../../etc/passwd'), whose string form starts with output/, so the second check passes as well.
4. FileResponse(abs_path) then hands the path to the OS, which resolves the traversal and reads the file outside the sandbox.

Fix

Resolve the path first, then verify it is strictly contained within one of the allow-listed directories using pathlib.Path.parents (not string prefix matching):

cwd = Path.cwd().resolve()
allowed_roots = [(cwd / prefix.rstrip('/')).resolve() for prefix in allowed_prefixes]

try:
    abs_path = (cwd / full_path).resolve(strict=False)
except (OSError, RuntimeError):
    raise HTTPException(status_code=403, detail="Access denied")

is_allowed = any(
    abs_path == root or root in abs_path.parents
    for root in allowed_roots
)
if not is_allowed:
    raise HTTPException(status_code=403, detail="Access denied: ...")

resolve() collapses .. segments and symlinks before the containment check, so any path that escapes the intended directories is rejected with 403 before FileResponse is called. The parents-based comparison also avoids the classic /tmp/output_evil vs /tmp/output prefix-collision bug that startswith would allow.

Proof of concept

Start the app locally (default bind is 0.0.0.0:8080) and request a traversal payload:

# Before the fix — leaks the host's /etc/passwd:
curl -sS 'http://127.0.0.1:8080/api/files/output/../../../../../etc/passwd'

# After the fix — returns 403:
curl -sS -o /dev/null -w '%{http_code}\n' \
     'http://127.0.0.1:8080/api/files/output/../../../../../etc/passwd'
# => 403

Legitimate requests to files under output/, workflows/, templates/, bgm/, data/bgm/, data/templates/, resources/ continue to work.

Testing

• Verified the traversal payload above returns 403 with the patched code and 200 (with file contents) without it.
• Verified requests for legitimate files inside each allow-listed prefix still return the file with the correct media type.
• Verified encoded variants (%2e%2e%2f...) are also rejected — FastAPI/Starlette decodes them before the handler sees the value, so they go through the same resolve() check.
• Verified non-existent legitimate paths still produce 404 (behaviour preserved).

Adversarial review

Before submitting we tried to disprove this. We checked whether the router or app installs any auth dependency that would gate the endpoint (grep -n 'dependencies=\|include_router' api/app.py api/routers/files.py) — none is present, and the default api_config.api_prefix is /api, so the route really is reachable unauthenticated. We also confirmed FastAPI's {file_path:path} converter does not strip .. segments, and that Path.cwd() / "output/../../../etc/passwd" is not normalized by pathlib without an explicit resolve() — matching the observed bypass of the original startswith check.

Scope

Single-file change, only touches the get_file handler; no API surface changes, no new dependencies.

---

_{Discovered by the Sebastion AI GitHub App.}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}