ly-wang19 435edbd8c6
fix(artifacts): offload blocking filesystem IO in artifact serving (#3551)
get_artifact ran its filesystem work directly on the event loop: virtual-path
resolution (os.path.abspath via .resolve()), exists/is_file probes, MIME sniffing
(mimetypes lazily stats the system MIME DB on first use), full-file
read_text/read_bytes, is_text_file_by_content (open+read), and .skill ZIP
open+extract. So serving any artifact blocked the loop for the whole read;
`make detect-blocking-io` flagged it. Same class as #3457 / #3529.

Offload each branch's IO via asyncio.to_thread: one sync helper per branch
(_load_skill_archive_member, _read_artifact_payload) folds stat + MIME + read /
extract into a single worker hop and returns a small (kind, mime, payload) plan
the handler turns into the response on the loop. FileResponse (download / active
content) keeps streaming the file itself. Behavior, branching, error codes, and
security boundaries are unchanged.

Add tests/blocking_io/test_artifacts_router.py anchor (text / binary / .skill
member), verified red->green under the strict Blockbuster gate. The gate also
caught a blocking call the static scan missed: resolve_thread_virtual_path's
.resolve() (os.path.abspath), now offloaded too.

Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-24 10:44:46 +08:00

230 lines
10 KiB
Python

import asyncio
import logging
import mimetypes
import zipfile
from pathlib import Path
from urllib.parse import quote
from fastapi import APIRouter, HTTPException, Request
from fastapi.responses import FileResponse, PlainTextResponse, Response
from app.gateway.authz import require_permission
from app.gateway.path_utils import resolve_thread_virtual_path
logger = logging.getLogger(__name__)
router = APIRouter(prefix="/api", tags=["artifacts"])
ACTIVE_CONTENT_MIME_TYPES = {
"text/html",
"application/xhtml+xml",
"image/svg+xml",
}
MAX_SKILL_ARCHIVE_MEMBER_BYTES = 16 * 1024 * 1024
_SKILL_ARCHIVE_READ_CHUNK_SIZE = 64 * 1024
def _build_content_disposition(disposition_type: str, filename: str) -> str:
"""Build an RFC 5987 encoded Content-Disposition header value."""
return f"{disposition_type}; filename*=UTF-8''{quote(filename)}"
def _build_attachment_headers(filename: str, extra_headers: dict[str, str] | None = None) -> dict[str, str]:
headers = {"Content-Disposition": _build_content_disposition("attachment", filename)}
if extra_headers:
headers.update(extra_headers)
return headers
def is_text_file_by_content(path: Path, sample_size: int = 8192) -> bool:
"""Check if file is text by examining content for null bytes."""
try:
with open(path, "rb") as f:
chunk = f.read(sample_size)
# Text files shouldn't contain null bytes
return b"\x00" not in chunk
except Exception:
return False
def _read_skill_archive_member(zip_ref: zipfile.ZipFile, info: zipfile.ZipInfo) -> bytes:
"""Read a .skill archive member while enforcing an uncompressed size cap."""
if info.file_size > MAX_SKILL_ARCHIVE_MEMBER_BYTES:
raise HTTPException(status_code=413, detail="Skill archive member is too large to preview")
chunks: list[bytes] = []
total_read = 0
with zip_ref.open(info, "r") as src:
while chunk := src.read(_SKILL_ARCHIVE_READ_CHUNK_SIZE):
total_read += len(chunk)
if total_read > MAX_SKILL_ARCHIVE_MEMBER_BYTES:
raise HTTPException(status_code=413, detail="Skill archive member is too large to preview")
chunks.append(chunk)
return b"".join(chunks)
def _extract_file_from_skill_archive(zip_path: Path, internal_path: str) -> bytes | None:
"""Extract a file from a .skill ZIP archive.
Args:
zip_path: Path to the .skill file (ZIP archive).
internal_path: Path to the file inside the archive (e.g., "SKILL.md").
Returns:
The file content as bytes, or None if not found.
"""
if not zipfile.is_zipfile(zip_path):
return None
try:
with zipfile.ZipFile(zip_path, "r") as zip_ref:
# List all files in the archive
infos_by_name = {info.filename: info for info in zip_ref.infolist()}
# Try direct path first
if internal_path in infos_by_name:
return _read_skill_archive_member(zip_ref, infos_by_name[internal_path])
# Try with any top-level directory prefix (e.g., "skill-name/SKILL.md")
for name, info in infos_by_name.items():
if name.endswith("/" + internal_path) or name == internal_path:
return _read_skill_archive_member(zip_ref, info)
# Not found
return None
except (zipfile.BadZipFile, KeyError):
return None
def _load_skill_archive_member(actual_skill_path: Path, skill_file_path: str, internal_path: str) -> tuple[bytes, str | None]:
"""Worker-thread body for the ``.skill`` branch of ``get_artifact``.
The ``exists`` / ``is_file`` probes, the ZIP open+extract, and the MIME
sniff (``mimetypes`` lazily stats the system MIME database on first use) are
blocking filesystem IO and must stay off the event loop. Raised
``HTTPException``s propagate through ``asyncio.to_thread`` unchanged,
preserving status codes.
"""
if not actual_skill_path.exists():
raise HTTPException(status_code=404, detail=f"Skill file not found: {skill_file_path}")
if not actual_skill_path.is_file():
raise HTTPException(status_code=400, detail=f"Path is not a file: {skill_file_path}")
content = _extract_file_from_skill_archive(actual_skill_path, internal_path)
if content is None:
raise HTTPException(status_code=404, detail=f"File '{internal_path}' not found in skill archive")
mime_type, _ = mimetypes.guess_type(internal_path)
return content, mime_type
def _read_artifact_payload(actual_path: Path, path: str, download: bool) -> tuple[str, str | None, bytes | str | None]:
"""Worker-thread body for the regular branch of ``get_artifact``.
Stat probes, MIME sniffing (``mimetypes`` lazily stats the system MIME
database on first use), and full-file reads are all blocking filesystem IO.
Returns a ``(kind, mime_type, payload)`` plan the handler turns into a
response on the loop: ``("file", mime, None)`` (let ``FileResponse`` stream
it), ``("text", mime, str)``, or ``("bytes", mime, bytes)``. Behavior/error
codes match the previous inline logic.
"""
if not actual_path.exists():
raise HTTPException(status_code=404, detail=f"Artifact not found: {path}")
if not actual_path.is_file():
raise HTTPException(status_code=400, detail=f"Path is not a file: {path}")
mime_type, _ = mimetypes.guess_type(actual_path)
# Active content / explicit download is streamed by FileResponse — no read here.
if download or mime_type in ACTIVE_CONTENT_MIME_TYPES:
return ("file", mime_type, None)
if mime_type and mime_type.startswith("text/"):
return ("text", mime_type, actual_path.read_text(encoding="utf-8"))
if is_text_file_by_content(actual_path):
return ("text", mime_type, actual_path.read_text(encoding="utf-8"))
return ("bytes", mime_type, actual_path.read_bytes())
@router.get(
"/threads/{thread_id}/artifacts/{path:path}",
summary="Get Artifact File",
description="Retrieve an artifact file generated by the AI agent. Text and binary files can be viewed inline, while active web content is always downloaded.",
)
@require_permission("threads", "read", owner_check=True)
async def get_artifact(thread_id: str, path: str, request: Request, download: bool = False) -> Response:
"""Get an artifact file by its path.
The endpoint automatically detects file types and returns appropriate content types.
Use the `download` query parameter to force file download for non-active content.
Args:
thread_id: The thread ID.
path: The artifact path with virtual prefix (e.g., mnt/user-data/outputs/file.txt).
request: FastAPI request object (automatically injected).
Returns:
The file content as a FileResponse with appropriate content type:
- Active content (HTML/XHTML/SVG): Served as download attachment
- Text files: Plain text with proper MIME type
- Binary files: Inline display with download option
Raises:
HTTPException:
- 400 if path is invalid or not a file
- 403 if access denied (path traversal detected)
- 404 if file not found
Query Parameters:
download (bool): If true, forces attachment download for file types that are
otherwise returned inline or as plain text. Active HTML/XHTML/SVG content
is always downloaded regardless of this flag.
Example:
- Get text file inline: `/api/threads/abc123/artifacts/mnt/user-data/outputs/notes.txt`
- Download file: `/api/threads/abc123/artifacts/mnt/user-data/outputs/data.csv?download=true`
- Active web content such as `.html`, `.xhtml`, and `.svg` artifacts is always downloaded
"""
# Check if this is a request for a file inside a .skill archive (e.g., xxx.skill/SKILL.md)
if ".skill/" in path:
# Split the path at ".skill/" to get the ZIP file path and internal path
skill_marker = ".skill/"
marker_pos = path.find(skill_marker)
skill_file_path = path[: marker_pos + len(".skill")] # e.g., "mnt/user-data/outputs/my-skill.skill"
internal_path = path[marker_pos + len(skill_marker) :] # e.g., "SKILL.md"
actual_skill_path = await asyncio.to_thread(resolve_thread_virtual_path, thread_id, skill_file_path)
# Offload the stat probes + ZIP open/extract + MIME sniff (blocking filesystem IO).
content, mime_type = await asyncio.to_thread(_load_skill_archive_member, actual_skill_path, skill_file_path, internal_path)
# Add cache headers to avoid repeated ZIP extraction (cache for 5 minutes)
cache_headers = {"Cache-Control": "private, max-age=300"}
download_name = Path(internal_path).name or actual_skill_path.stem
if download or mime_type in ACTIVE_CONTENT_MIME_TYPES:
return Response(content=content, media_type=mime_type or "application/octet-stream", headers=_build_attachment_headers(download_name, cache_headers))
if mime_type and mime_type.startswith("text/"):
return PlainTextResponse(content=content.decode("utf-8"), media_type=mime_type, headers=cache_headers)
# Default to plain text for unknown types that look like text
try:
return PlainTextResponse(content=content.decode("utf-8"), media_type="text/plain", headers=cache_headers)
except UnicodeDecodeError:
return Response(content=content, media_type=mime_type or "application/octet-stream", headers=cache_headers)
actual_path = await asyncio.to_thread(resolve_thread_virtual_path, thread_id, path)
logger.info(f"Resolving artifact path: thread_id={thread_id}, requested_path={path}, actual_path={actual_path}")
# Offload path stat + MIME sniff + file reads (all blocking filesystem IO).
# Active content and explicit downloads are streamed by FileResponse, so the
# worker only reports the kind; inline text/binary payloads are read in-thread.
kind, mime_type, payload = await asyncio.to_thread(_read_artifact_payload, actual_path, path, download)
if kind == "file":
# Always force download for active content types to prevent script
# execution in the application origin when users open generated artifacts.
return FileResponse(path=actual_path, filename=actual_path.name, media_type=mime_type, headers=_build_attachment_headers(actual_path.name))
if kind == "text":
return PlainTextResponse(content=payload, media_type=mime_type)
return Response(content=payload, media_type=mime_type, headers={"Content-Disposition": _build_content_disposition("inline", actual_path.name)})