fix: sanitize upload filename to prevent path traversal (#638)

[swaylq]

Summary

Fixes #638 — an arbitrary file write/delete in the ChatDev workflow server.

AttachmentService.save_upload_file (server/services/attachment_service.py) built the upload's temp path by joining the client-supplied multipart filename onto a fresh temp directory with no sanitization:

filename = upload.filename or "upload.bin"
temp_dir = Path(tempfile.mkdtemp(prefix="mac_upload_"))
temp_path = temp_dir / filename          # filename attacker-controlled
with temp_path.open("wb") as buffer:     # arbitrary write
    ...
finally:
    temp_path.unlink()                    # arbitrary delete

A filename like ../../../path/to/victim escapes the temp directory: the upload writes attacker bytes to that path and the finally cleanup then unlinks it. register_file basenames the name after the write, so it doesn't mitigate this. The endpoint (POST /api/uploads/{session_id}) only needs a known session id, which is minted without authentication.

Fix

Reduce the filename to its basename before building the temp path — normalising both POSIX (/) and Windows (\) separators and falling back to upload.bin for empty / . / ... The write target now always stays inside the mkdtemp directory.

candidate = os.path.basename((raw or "").replace("\\", "/")).strip()
if not candidate or candidate in {".", ".."}:
    return "upload.bin"
return candidate

Tests

tests/test_attachment_upload_filename.py:

• Parametrized coverage of the basename normalisation (traversal, absolute, Windows-style, empty/./..).
• End-to-end test: calls the real save_upload_file with a traversal filename and asserts a victim file outside the temp/WareHouse area is not overwritten or deleted, and that the stored record uses the sanitized basename.

Verified the end-to-end test fails without the fix (victim file is deleted) and passes with it. Full file: 12 passed.

---

🤖 AI-assisted contribution (Claude Code). Fix and tests reviewed and verified locally.