diff --git a/error-code-baseline.json b/error-code-baseline.json index 8a1e144c2ee..71a22aac323 100644 --- a/error-code-baseline.json +++ b/error-code-baseline.json @@ -5,7 +5,7 @@ "opaque_body": 18, "dynamic_status": 43 }, - "_compliant": 1389, + "_compliant": 1410, "files": { "apps/builtins/auto_research/handlers.py": { "dynamic_status": 1, diff --git a/src/kiro_crew/dashboard/handlers/__init__.py b/src/kiro_crew/dashboard/handlers/__init__.py index 91e6faa3b04..275d31e2450 100644 --- a/src/kiro_crew/dashboard/handlers/__init__.py +++ b/src/kiro_crew/dashboard/handlers/__init__.py @@ -136,6 +136,7 @@ def sel(): api_dashboard_config, api_file_diff, api_file_download, + api_file_office_preview, api_file_raw, api_file_read, api_file_search, diff --git a/src/kiro_crew/dashboard/handlers/files.py b/src/kiro_crew/dashboard/handlers/files.py index 7dcba84d6dd..d70d8526442 100644 --- a/src/kiro_crew/dashboard/handlers/files.py +++ b/src/kiro_crew/dashboard/handlers/files.py @@ -36,6 +36,7 @@ from kiro_crew.dashboard.file_index import _SKIP_DIRS as _WALK_SKIP_DIRS from kiro_crew.dashboard.handlers._shared import _probe_persisted_session from kiro_crew.dashboard.state import DashboardState +from kiro_crew.doc_parser import extract_text from kiro_crew.hooks import FileTooLargeError, safe_read_file_bytes, safe_read_prefix from kiro_crew.messaging import upload_gate from kiro_crew.messaging.display_safety import redact_for_display @@ -2339,6 +2340,241 @@ async def api_file_download(request: web.Request) -> web.Response: ) +# Extensions previewable via kiro_crew.doc_parser (OOXML docx/pptx). Legacy +# binary formats (.doc, .ppt), the OpenDocument family (.odt/.ods/.odp), and +# spreadsheet formats (.xls/.xlsx) fall through to the download card because +# doc_parser only understands ZIP+XML OOXML, and adding openpyxl or a legacy +# OLE reader would grow the dependency tree noticeably for a preview feature. +_OFFICE_PREVIEWABLE_EXT = {".docx", ".pptx"} +# Cap the returned text so a huge .docx doesn't blow the JSON payload / DOM. +# Mirrors api_file_read's 512 KB read cap. Anything larger is truncated and +# the frontend shows a "Download for full contents" affordance. +_OFFICE_PREVIEW_CAP = 512_000 + + +class _PreviewUnsupported(Exception): + """The validated path's extension is outside :data:`_OFFICE_PREVIEWABLE_EXT`. + + Endpoint-local, mirroring :class:`_SheetRefusal`: ``_OpenDenied``'s codes + are the SHARED file-serving boundary's vocabulary, and this is this + endpoint's own FORMAT policy rather than a security refusal, so it does + not belong in that enum. Raised from inside the worker callback so the + checked file object is closed by its ``with`` block on the same thread. + """ + + +async def api_file_office_preview(request: web.Request) -> web.Response: + """GET /api/file-office-preview?path=... — extract inline text preview from a .docx/.pptx. + + Sibling of /api/file-download. file-download streams original bytes for + saving to disk; this endpoint returns plaintext extracted from the + OOXML XML inside so the dashboard can render a scrollable preview of + the document contents in place of the "can't view a binary" download + card — a common ask for anyone browsing shared reports in the file + tree without wanting to save each one. + + Uses ``kiro_crew.doc_parser.extract_text`` which parses the .docx / + .pptx ZIP+XML with hardened defusedxml (XXE-safe) and returns "" on + any failure. python-docx / python-pptx are not required. + + Not supported (fall through to download): .doc, .ppt, .xls, .xlsx, + .odt, .ods, .odp. The frontend keeps the download card for these. + + Security: the open-and-check prefix is the SHARED + :func:`_open_checked_file` (dashboard path validation, sensitive-path + block, is-file, symlink-refusing ``_open_rb_nofollow`` — atomic + O_NOFOLLOW on POSIX, lstat guard on Windows — then fstat), never a + hand-rolled second spelling of it, so a future hardening change to that + boundary lands here too. This endpoint's own POLICY on top is the 50 MB + ``fstat_cap``, the ``.docx``/``.pptx`` format gate, the aggregate + extraction budget, and credential redaction before the preview cap is + applied. All of it — validation, open, fstat, ZIP+XML parsing, + redaction — runs in ONE worker-thread hop, like ``api_file_sheet``. + """ + raw_path = request.query.get("path", "") + + def _log(outcome: str, res: str, error: str = "") -> None: + kw = {"error": error} if error else {} + _sel().log_tool_invocation( + session_key="dashboard", tool_name="file_office_preview", + outcome=outcome, resources=res, **kw, + ) + + # Resolve relative paths against project dir when resolve=1. Uses the + # shared helper (same as api_file_read / api_file_download / file-raw): + # it passes Windows-absolute/UNC shapes through to the validator, whose + # network-path gate runs BEFORE realpath — never re-implement this inline. + if request.query.get("resolve") == "1": + raw_path, _resolve_err = _resolve_project_relative(raw_path) + if _resolve_err == "cannot_resolve": + _log("denied", request.query.get("path", ""), "cannot_resolve") + return web.json_response( + {"error": "cannot resolve: no project dir configured", "code": "no_project_dir"}, + status=400, + ) + if _resolve_err == "outside_project": + _log("denied", request.query.get("path", ""), "outside_project") + return web.json_response( + {"error": "path outside project directory", "code": "path_outside_project"}, + status=400, + ) + + try: + validate_tool_args({"path": raw_path}, FILE_READ_SCHEMA) + except ValidationError: + _log("denied", raw_path) + return web.json_response({"error": "invalid input", "code": "invalid_input"}, status=400) + + # The validated path once the shared prefix produces one -- exported by + # the worker callback so the exception handlers log the same SEL resource + # the success path does. + res_path = raw_path + + def _open_and_extract() -> dict[str, object] | _OpenDenied: + """Open-and-check plus extract, in ONE worker-thread hop. + + Everything here is blocking I/O or CPU-bound — realpath validation, + the sensitive-path screen, the open, the fstat, ZIP decompression, + XML parsing, redaction — so none of it may run on the event loop: an + NFS/FUSE-backed document makes even the validate/open envelope block + for seconds, stalling every session's streaming and the liveness + heartbeat. + + The checked open file object never crosses back to the event loop: + every path that opens it also closes it on THIS thread (refusals + close inside the prefix; the ``with`` block below covers the rest, + the format refusal included). A cancellation of the awaiting task + therefore cannot strand an open file in a discarded future or + finalize one on the loop — the future's result is only ever a + payload dict or a typed refusal. + """ + nonlocal res_path + # fstat_cap is this endpoint's size gate, enforced on the fd BEFORE + # any ZIP parsing: zipfile.ZipFile materializes the archive's central + # directory in memory, bounded only by the file itself, so a crafted + # archive could otherwise exhaust memory before doc_parser's + # per-entry and aggregate budgets ever apply. Same 50 MB ceiling as + # file uploads. log_open_failure=False: this endpoint answers a coded + # refusal, so a request loop against a known-unreadable path cannot + # amplify into the log. + checked = _open_checked_file( + raw_path, + tool_name="file_office_preview", + fstat_cap=_MAX_UPLOAD_BYTES, + log_open_failure=False, + ) + if isinstance(checked, _OpenDenied): + return checked + res_path = checked.path + with checked.file as fobj: + if os.path.splitext(checked.path)[1].lower() not in _OFFICE_PREVIEWABLE_EXT: + raise _PreviewUnsupported(checked.path) + # extract_text parses through the SAME handle the prefix opened + # and fstat-ed (its opt-in fileobj parameter), so the bytes + # parsed are exactly the bytes measured — no stat→open TOCTOU + # window. max_chars bounds AGGREGATE extraction (cap + 1 keeps + # the truncation flag detectable): a deck with thousands of + # slides stops parsing at the budget instead of accumulating + # unbounded text. It never raises — returns "" on any failure. + text = extract_text( + checked.path, + filename=os.path.basename(checked.path), + max_chars=_OFFICE_PREVIEW_CAP + 1, + fileobj=fobj, + ) + truncated = len(text) > _OFFICE_PREVIEW_CAP + # Redact BEFORE truncating: slicing first could cut a credential + # across the cap boundary, leaving an unmatched prefix the redactor + # no longer recognizes. Redaction may change the length, so the + # truncation flag is computed from the raw extraction above. + text = redact(text) + if truncated: + text = text[:_OFFICE_PREVIEW_CAP] + return { + "text": text, + "truncated": truncated, + # No `empty` field: doc_parser returns "" for both a genuinely + # blank document and a parse failure, so the two are + # indistinguishable here. The frontend treats empty `text` as + # "no preview available" and falls back to the download card. + } + + try: + result = await asyncio.to_thread(_open_and_extract) + except asyncio.CancelledError: + # Gateway shutdown / client disconnect while the worker thread is + # parsing: the access attempt already happened, so record it before + # propagating — CancelledError is a BaseException and would bypass + # the Exception handler below, leaving the access unaudited. No + # resource handling here: the worker callback owns the file's whole + # lifetime. + _log("cancelled", res_path) + raise + except _PreviewUnsupported: + # 415 (not 400) so the frontend can distinguish "unsupported format, + # keep showing the download card" from "invalid input, something's + # actually wrong". The frontend short-circuits known-unsupported + # extensions client-side, so this branch is the safety net (direct + # API calls, frontend/backend list drift). + _log("denied", res_path, "unsupported_preview_format") + return web.json_response( + { + "error": "unsupported format for inline preview", + "code": "unsupported_preview_format", + }, + status=415, + ) + except Exception: # noqa: BLE001 # last-resort guard; doc_parser already logs + logger.exception("file_office_preview extract_text failed for %s", res_path) + _log("failure", res_path) + return web.json_response( + {"error": "failed to extract preview", "code": "preview_extraction_failed"}, + status=500, + ) + if isinstance(result, _OpenDenied): + # The shared prefix's typed refusals, mapped onto this endpoint's SEL + # outcomes and response vocabulary — the part that legitimately + # differs per endpoint. + code, res = result.code, result.path + if code == "invalid_path": + _log("denied", res) + return web.json_response( + {"error": "invalid or forbidden path", "code": "forbidden_path"}, status=400, + ) + if code == "sensitive_path": + _log("denied", res, "sensitive_path") + return web.json_response( + {"error": "sensitive path blocked", "code": "sensitive_path"}, status=403, + ) + if code == "not_found": + _log("not_found", res) + return web.json_response({"error": "not found", "code": "not_found"}, status=404) + if code == "symlink_refused": + _log("denied", res, "symlink_rejected") + return web.json_response( + {"error": "symlinks not allowed", "code": "symlink_rejected"}, status=403, + ) + if code == "file_too_large": + _log("denied", res, "file_too_large") + return web.json_response( + { + "error": ( + "file too large for preview " + f"(max {_MAX_UPLOAD_BYTES // 1024 // 1024}MB)" + ), + "code": "file_too_large", + }, + status=413, + ) + # read_failed: the residual code. + _log("failure", res) + return web.json_response( + {"error": "cannot read file", "code": "file_read_failed"}, status=500, + ) + _log("success", res_path) + return web.json_response(result) + + async def api_file_raw(request: web.Request) -> web.Response: """GET /api/file-raw?path=... — serve a file with its native content type (images, etc.).""" # Envelope (validate -> sensitive -> nofollow-open -> bounded read) is diff --git a/src/kiro_crew/dashboard/routes/taskrunner.py b/src/kiro_crew/dashboard/routes/taskrunner.py index b5b5ba740e7..8bafb3e1dd2 100644 --- a/src/kiro_crew/dashboard/routes/taskrunner.py +++ b/src/kiro_crew/dashboard/routes/taskrunner.py @@ -37,6 +37,7 @@ def register(app: web.Application) -> None: app.router.add_post("/api/reveal", handlers.api_reveal_path) app.router.add_get("/api/file-read", handlers.api_file_read) app.router.add_get("/api/file-download", handlers.api_file_download) + app.router.add_get("/api/file-office-preview", handlers.api_file_office_preview) app.router.add_get("/api/file-raw", handlers.api_file_raw) app.router.add_get("/api/file-stream", handlers.api_file_stream) app.router.add_get("/api/file-watch", handlers.api_file_watch) diff --git a/src/kiro_crew/doc_parser.py b/src/kiro_crew/doc_parser.py index b958f70a274..91d88c4de41 100644 --- a/src/kiro_crew/doc_parser.py +++ b/src/kiro_crew/doc_parser.py @@ -17,10 +17,12 @@ from __future__ import annotations import logging +import os import re import zipfile import zlib from pathlib import Path +from typing import IO # Optional so a stale install (git pull without `pip install -e .`) degrades # to "docx/pptx parsing unavailable" instead of killing every CLI entry at @@ -33,7 +35,12 @@ from kiro_crew.security import is_sensitive_path from kiro_crew.sel import sel -from kiro_crew.zip_vet import ZipInventoryRejected, vet_zip_inventory +from kiro_crew.zip_vet import ( + TAIL_WINDOW, + ZipInventoryRejected, + vet_zip_inventory, + vet_zip_inventory_bytes, +) logger = logging.getLogger(__name__) @@ -72,11 +79,33 @@ def is_parseable_document(mimetype: str = "", filename: str = "") -> bool: return ext in DOC_EXTENSIONS -def extract_text(path: str, mimetype: str = "", filename: str = "") -> str: +def extract_text( + path: str, + mimetype: str = "", + filename: str = "", + max_chars: int | None = None, + fileobj: IO[bytes] | None = None, +) -> str: """Extract readable text from a document file. Detects format from *mimetype* first, then falls back to file extension. Returns empty string on any failure. + + *max_chars*, when given, bounds the AGGREGATE extracted text: parsing + stops as soon as at least that many characters have been collected (the + result may slightly overshoot, callers truncate to their exact cap). + Without it a multi-part container (e.g. a .pptx with thousands of + slides, each under the per-entry decompression cap) could accumulate + unbounded text in memory. Callers that only need a preview should pass + their cap + 1 so truncation stays detectable. + + *fileobj*, when given, is an ALREADY-OPEN binary file the .docx/.pptx + ZIP is read from instead of re-opening *path* — callers that stat-gate + the file first pass the same handle so the bytes parsed are exactly the + bytes measured (no stat→open TOCTOU window). *path* is still used for + sensitive-path screening, format detection and logging. PDF extraction + is byte-scan based and still reads *path*; no current fileobj caller + requests PDFs. """ if is_sensitive_path(path): logger.warning("Refusing to read sensitive path: %s", path) @@ -104,9 +133,9 @@ def extract_text(path: str, mimetype: str = "", filename: str = "") -> str: return "" try: if fmt == "docx": - return _extract_docx(path) + return _extract_docx(path, max_chars=max_chars, fileobj=fileobj) if fmt == "pptx": - return _extract_pptx(path) + return _extract_pptx(path, max_chars=max_chars, fileobj=fileobj) if fmt == "pdf": return _extract_pdf(path) except Exception: @@ -128,21 +157,51 @@ def _safe_decompress(data: bytes, max_size: int | None = None) -> bytes: return result -def _vet_archive_inventory(path: str) -> bool: +def _vet_archive_inventory(path: str, fileobj: IO[bytes] | None = None) -> bool: """Preflight an OOXML container's declared inventory before opening it. Returns True when the archive is within bounds. Fails closed: a rejected or unreadable tail returns False, and the caller degrades to "" like every other unreadable-document path in this module. + + When *fileobj* is given the tail is read from THAT handle, not from *path*: + the handle is what ``zipfile`` will parse, so vetting the path instead + would bound a different archive than the one opened -- a swapped path + between the two reads would let an over-cap inventory reach the allocation + this vet exists to prevent. The position is restored so the caller's + ``ZipFile`` still sees the whole file. """ try: - vet_zip_inventory(path, max_members=_MAX_ARCHIVE_MEMBERS) + if fileobj is not None: + tail = _read_tail_from(fileobj) + vet_zip_inventory_bytes(tail, max_members=_MAX_ARCHIVE_MEMBERS) + else: + vet_zip_inventory(path, max_members=_MAX_ARCHIVE_MEMBERS) except ZipInventoryRejected as exc: logger.warning("archive inventory rejected (%s)", exc.reason) return False + except OSError as exc: + # An unseekable or unreadable handle is an unvettable archive, and this + # guard fails closed like the path branch does. + logger.warning("cannot read archive tail from handle: %s", exc) + return False return True +def _read_tail_from(fileobj: IO[bytes]) -> bytes: + """Read the EOCD search window from an already-open archive handle. + + Mirrors :func:`kiro_crew.zip_vet._read_tail` for the fd-based caller, then + rewinds so the handle is still at byte 0 for ``zipfile.ZipFile``. + """ + try: + size = fileobj.seek(0, os.SEEK_END) + fileobj.seek(max(0, size - TAIL_WINDOW)) + return fileobj.read(TAIL_WINDOW) + finally: + fileobj.seek(0) + + def _read_zip_entry( zf: zipfile.ZipFile, name: str, max_size: int | None = None, ) -> bytes | None: @@ -166,7 +225,9 @@ def _read_zip_entry( _W_NS = "{http://schemas.openxmlformats.org/wordprocessingml/2006/main}" -def _extract_docx(path: str) -> str: +def _extract_docx( + path: str, max_chars: int | None = None, fileobj: IO[bytes] | None = None, +) -> str: """Extract text from a .docx file (ZIP containing word/document.xml). Must only be called from extract_text() which enforces is_sensitive_path(). @@ -174,10 +235,11 @@ def _extract_docx(path: str) -> str: assert _xml_fromstring is not None # extract_text() gates the None case if is_sensitive_path(path): return "" - if not _vet_archive_inventory(path): + if not _vet_archive_inventory(path, fileobj): return "" paragraphs: list[str] = [] - with zipfile.ZipFile(path, "r") as zf: + collected = 0 + with zipfile.ZipFile(fileobj if fileobj is not None else path, "r") as zf: if "word/document.xml" not in zf.namelist(): return "" data = _read_zip_entry(zf, "word/document.xml") @@ -190,7 +252,19 @@ def _extract_docx(path: str) -> str: if t_elem.text: texts.append(t_elem.text) if texts: - paragraphs.append("".join(texts)) + joined = "".join(texts) + if paragraphs: + # Count the "\n" separator only BETWEEN paragraphs, so + # `collected` tracks len("\n".join(paragraphs)) exactly. + # Charging the first paragraph a separator too let a + # cap-sized opening paragraph stop extraction while the + # caller's length check read the result as un-truncated, + # silently dropping the rest of the document. + collected += 1 + paragraphs.append(joined) + collected += len(joined) + if max_chars is not None and collected >= max_chars: + break return "\n".join(paragraphs) @@ -200,18 +274,25 @@ def _extract_docx(path: str) -> str: _SLIDE_RE = re.compile(r"^ppt/slides/slide(\d+)\.xml$") -def _extract_pptx(path: str) -> str: +def _extract_pptx( + path: str, max_chars: int | None = None, fileobj: IO[bytes] | None = None, +) -> str: """Extract text from a .pptx file (ZIP containing ppt/slides/*.xml). Must only be called from extract_text() which enforces is_sensitive_path(). + + With *max_chars* set, slide iteration stops as soon as the collected + text meets the budget — later slides are never decompressed or parsed, + so a deck with thousands of slides cannot accumulate unbounded text. """ assert _xml_fromstring is not None # extract_text() gates the None case if is_sensitive_path(path): return "" - if not _vet_archive_inventory(path): + if not _vet_archive_inventory(path, fileobj): return "" slides: list[tuple[int, str]] = [] - with zipfile.ZipFile(path, "r") as zf: + collected = 0 + with zipfile.ZipFile(fileobj if fileobj is not None else path, "r") as zf: slide_names = sorted( (n for n in zf.namelist() if _SLIDE_RE.match(n)), key=lambda n: int(_SLIDE_RE.match(n).group(1)), # type: ignore[union-attr] @@ -227,7 +308,11 @@ def _extract_pptx(path: str) -> str: if t_elem.text: texts.append(t_elem.text) if texts: - slides.append((num, "\n".join(texts))) + slide_text = "\n".join(texts) + slides.append((num, slide_text)) + collected += len(slide_text) + if max_chars is not None and collected >= max_chars: + break parts: list[str] = [] for num, text in slides: parts.append(f"--- Slide {num} ---\n{text}") diff --git a/test/test_ci_surface_tests.py b/test/test_ci_surface_tests.py index 9c2d01a549e..7520f516bf0 100644 --- a/test/test_ci_surface_tests.py +++ b/test/test_ci_surface_tests.py @@ -277,6 +277,7 @@ def test_ignore_list_matches_the_names_conftest_previously_inlined() -> None: "test_webapp_preview.py", "test_file_raw.py", "test_file_download.py", + "test_file_office_preview.py", "test_dashboard_file_io.py", "test_dev_fleet_app.py", } diff --git a/test/test_doc_parser.py b/test/test_doc_parser.py index 905e484f047..5ceadcb2779 100644 --- a/test/test_doc_parser.py +++ b/test/test_doc_parser.py @@ -442,3 +442,110 @@ def test_pptx_is_bounded_too(self, tmp_path, monkeypatch): z.writestr(f"ppt/media/f{i}.bin", b"x") monkeypatch.setattr(doc_parser, "_MAX_ARCHIVE_MEMBERS", 5) assert doc_parser._extract_pptx(str(path)) == "" + + +# ── Aggregate extraction budget (max_chars) ── + + +class TestMaxCharsBudget: + """extract_text(max_chars=...) bounds AGGREGATE retained text. + + Added for the office-preview endpoint: a .pptx with thousands of slides, + each under the per-entry decompression cap, must not accumulate unbounded + text. Budget met => later slides are never decompressed or parsed. + """ + + def test_pptx_budget_stops_slide_iteration(self): + path = _make_pptx([["s" * 1000] for _ in range(50)]) + try: + text = extract_text(path, filename="deck.pptx", max_chars=3000) + finally: + os.unlink(path) + assert len(text) < 10 * 1000, "budget must bound aggregate extraction" + assert "--- Slide 1 ---" in text + assert "--- Slide 50 ---" not in text + + def test_docx_budget_bounds_paragraphs(self): + path = _make_docx(["p" * 1000 for _ in range(50)]) + try: + text = extract_text(path, filename="long.docx", max_chars=3000) + finally: + os.unlink(path) + assert len(text) < 10 * 1000 + + def test_no_budget_extracts_everything(self): + """Without max_chars behavior is unchanged (existing callers unaffected).""" + path = _make_pptx([["alpha"], ["beta"], ["gamma"]]) + try: + text = extract_text(path, filename="deck.pptx") + finally: + os.unlink(path) + assert "alpha" in text and "beta" in text and "gamma" in text + + def test_docx_cap_sized_first_paragraph_does_not_stop_extraction(self): + """Separator accounting must not charge the FIRST paragraph. + + Regression: `collected += len(joined) + 1` counted a "\n" separator + for the first paragraph too, so a document whose opening paragraph + exactly filled the budget stopped extraction there — the caller's + length check then read the result as un-truncated and the rest of + the document was silently dropped. + """ + first = "a" * 100 + path = _make_docx([first, "TAIL-MARKER"]) + try: + # Budget = len(first) + 1: previously the first paragraph alone + # met it (100 + phantom separator); now it is 100 < 101, so + # extraction must continue into the second paragraph. + text = extract_text(path, filename="doc.docx", max_chars=101) + finally: + os.unlink(path) + assert "TAIL-MARKER" in text, "cap-sized first paragraph dropped the rest" + # And the returned length now exceeds the budget, so a caller + # passing cap+1 sees the truncation instead of missing content. + assert len(text) > 101 + + +class TestFileobjExtraction: + """extract_text(fileobj=...) parses the already-open handle. + + Added for the office-preview endpoint's open-once discipline: the + handler fstat-gates the size on an O_NOFOLLOW fd and hands the SAME + handle here, so the bytes parsed are exactly the bytes measured (no + stat→open TOCTOU window). + """ + + def test_docx_fileobj_matches_path_result(self): + path = _make_docx(["hello", "world"]) + try: + via_path = extract_text(path, filename="a.docx") + with open(path, "rb") as f: + via_fileobj = extract_text(path, filename="a.docx", fileobj=f) + finally: + os.unlink(path) + assert via_fileobj == via_path == "hello\nworld" + + def test_pptx_fileobj_matches_path_result(self): + path = _make_pptx([["alpha"], ["beta"]]) + try: + via_path = extract_text(path, filename="d.pptx") + with open(path, "rb") as f: + via_fileobj = extract_text(path, filename="d.pptx", fileobj=f) + finally: + os.unlink(path) + assert via_fileobj == via_path + assert "alpha" in via_fileobj and "beta" in via_fileobj + + def test_fileobj_is_read_instead_of_path(self): + """The handle's bytes win — proves parsing never re-opens the path.""" + real = _make_docx(["FROM-FILEOBJ"]) + decoy = _make_docx(["FROM-PATH"]) + try: + with open(real, "rb") as f: + # Pass the DECOY path with the REAL handle: a re-open of the + # path would surface the decoy text. + text = extract_text(decoy, filename="a.docx", fileobj=f) + finally: + os.unlink(real) + os.unlink(decoy) + assert text == "FROM-FILEOBJ" diff --git a/test/test_file_office_preview.py b/test/test_file_office_preview.py new file mode 100644 index 00000000000..d9009cd36a9 --- /dev/null +++ b/test/test_file_office_preview.py @@ -0,0 +1,320 @@ +"""Tests for /api/file-office-preview — inline text extraction for .docx/.pptx. + +Pins the endpoint's security envelope on the enforcing side (sensitive-path +403, unsupported-format 415 with SEL audit, resolve=1 via the shared +_resolve_project_relative helper) plus the response contract the frontend +relies on (text/truncated, no format/supported/empty fields) and the +aggregate extraction budget (a many-slide deck cannot accumulate unbounded +text — doc_parser stops at the caller's cap). +""" + +from __future__ import annotations + +import asyncio +import os +import threading +import zipfile +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest +from aiohttp import web +from aiohttp.test_utils import TestClient, TestServer, make_mocked_request + +from kiro_crew.dashboard.handlers import api_file_office_preview +from kiro_crew.dashboard.handlers import files as files_mod +from kiro_crew.dashboard.handlers.files import _MAX_UPLOAD_BYTES, _OFFICE_PREVIEW_CAP + + +def _make_app() -> web.Application: + app = web.Application() + app.router.add_get("/api/file-office-preview", api_file_office_preview) + return app + + +@pytest.fixture +def mock_sel(): + with ( + patch("kiro_crew.sel.sel") as m, + patch("kiro_crew.dashboard.handlers.files.is_sensitive_path", return_value=False), + ): + instance = MagicMock() + m.return_value = instance + yield instance + + +def _write_docx(path: str, paragraphs: list[str]) -> None: + body = "\n".join(f"{p}" for p in paragraphs) + xml = ( + '' + '' + f"{body}" + ) + with zipfile.ZipFile(path, "w") as zf: + zf.writestr("word/document.xml", xml) + + +# --- Happy path: contract the frontend relies on --- + + +@pytest.mark.asyncio +async def test_docx_preview_returns_text_and_truncated_only(tmp_path, mock_sel): + f = tmp_path / "report.docx" + _write_docx(str(f), ["Introduction", "First paragraph of the document."]) + with patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get(f"/api/file-office-preview?path={f}") + assert resp.status == 200 + body = await resp.json() + assert "Introduction" in body["text"] + assert body["truncated"] is False + # Zero-consumer fields must NOT come back (review: dropped surface). + assert "format" not in body + assert "supported" not in body + assert "empty" not in body + + +@pytest.mark.asyncio +async def test_truncation_flag_set_and_text_capped(tmp_path, mock_sel): + f = tmp_path / "huge.docx" + # One paragraph larger than the cap: extraction budget (cap + 1) keeps + # the truncation detectable while the response text is cut to the cap. + _write_docx(str(f), ["x" * (_OFFICE_PREVIEW_CAP + 100)]) + with patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get(f"/api/file-office-preview?path={f}") + assert resp.status == 200 + body = await resp.json() + assert body["truncated"] is True + assert len(body["text"]) == _OFFICE_PREVIEW_CAP + + +# --- Security envelope --- + + +@pytest.mark.asyncio +async def test_unsupported_extension_415_with_sel_audit(tmp_path, mock_sel): + f = tmp_path / "legacy.xls" + f.write_bytes(b"\xd0\xcf\x11\xe0old-ole-junk") + with patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get(f"/api/file-office-preview?path={f}") + assert resp.status == 415 + body = await resp.json() + assert body["code"] == "unsupported_preview_format" + # The denial must leave an SEL record (review: audit gap). + denied = [ + c + for c in mock_sel.log_tool_invocation.call_args_list + if c.kwargs.get("outcome") == "denied" + and c.kwargs.get("error") == "unsupported_preview_format" + ] + assert denied, "unsupported-format 415 must be SEL-audited" + + +@pytest.mark.asyncio +async def test_sensitive_path_403(tmp_path, mock_sel): + f = tmp_path / "secrets.docx" + _write_docx(str(f), ["top secret"]) + with ( + patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)), + patch( + "kiro_crew.dashboard.handlers.files.is_sensitive_path", + return_value=True, + ), + ): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get(f"/api/file-office-preview?path={f}") + assert resp.status == 403 + body = await resp.json() + assert body["code"] == "sensitive_path" + + +@pytest.mark.asyncio +async def test_forbidden_path_400(mock_sel): + with patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=None): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get("/api/file-office-preview?path=/etc/passwd.docx") + assert resp.status == 400 + body = await resp.json() + assert body["code"] == "forbidden_path" + + +@pytest.mark.asyncio +async def test_resolve_uses_shared_helper(tmp_path, mock_sel): + """resolve=1 goes through _resolve_project_relative (review: no inline copy).""" + f = tmp_path / "proj" / "doc.docx" + f.parent.mkdir() + _write_docx(str(f), ["hello from project"]) + with ( + patch.dict(os.environ, {"KIROCREW_PROJECT_DIR": str(f.parent)}), + patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)), + ): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get("/api/file-office-preview?path=doc.docx&resolve=1") + assert resp.status == 200 + body = await resp.json() + assert "hello from project" in body["text"] + + +@pytest.mark.asyncio +async def test_resolve_outside_project_denied_and_audited(tmp_path, mock_sel): + proj = tmp_path / "proj" + proj.mkdir() + with patch.dict(os.environ, {"KIROCREW_PROJECT_DIR": str(proj)}): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get("/api/file-office-preview?path=../outside.docx&resolve=1") + assert resp.status == 400 + body = await resp.json() + assert body["code"] == "path_outside_project" + denied = [ + c + for c in mock_sel.log_tool_invocation.call_args_list + if c.kwargs.get("outcome") == "denied" and c.kwargs.get("error") == "outside_project" + ] + assert denied, "resolve denial must be SEL-audited" + + +@pytest.mark.asyncio +async def test_oversized_file_413_before_any_parsing(tmp_path, mock_sel): + """The size gate runs BEFORE zipfile ever opens the archive. + + The gate fstats the already-open O_NOFOLLOW fd (not the path), so the + oversize is a real sparse file: a stat-the-path mock would no longer + reach the code under test. + """ + f = tmp_path / "huge.docx" + _write_docx(str(f), ["small real content"]) + os.truncate(str(f), 51 * 1024 * 1024) # sparse: st_size > cap, no disk cost + with patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get(f"/api/file-office-preview?path={f}") + assert resp.status == 413 + body = await resp.json() + assert body["code"] == "file_too_large" + denied = [ + c + for c in mock_sel.log_tool_invocation.call_args_list + if c.kwargs.get("outcome") == "denied" and c.kwargs.get("error") == "file_too_large" + ] + assert denied, "oversized preview request must be SEL-audited" + + +@pytest.mark.asyncio +async def test_extraction_reads_through_the_prefix_fd_and_closes_it(tmp_path, mock_sel): + """No stat→open TOCTOU, and the checked fd never crosses back to the loop. + + extract_text must receive the SAME open handle the shared prefix opened + and fstat-ed (never re-open the path), and that handle must already be + closed once the response is built: every path that opens it also closes + it on the worker thread, so a cancellation cannot strand an open file in + a discarded future or finalize one on the event loop. + """ + f = tmp_path / "doc.docx" + _write_docx(str(f), ["content"]) + seen: dict[str, Any] = {} + real_extract = files_mod.extract_text + + def _spy(path, **kwargs): + seen["fileobj"] = kwargs.get("fileobj") + return real_extract(path, **kwargs) + + with ( + patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)), + patch("kiro_crew.dashboard.handlers.files.extract_text", side_effect=_spy), + ): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get(f"/api/file-office-preview?path={f}") + assert resp.status == 200 + body = await resp.json() + assert body["text"] == "content" + fobj = seen["fileobj"] + assert fobj is not None, "extract_text must be handed the prefix's checked fd" + assert fobj.closed, "the checked file object must be closed on the worker thread" + + +@pytest.mark.asyncio +async def test_open_envelope_is_the_shared_prefix_and_runs_off_the_loop(tmp_path, mock_sel): + """The validate→sensitive→isfile→open→fstat envelope is shared AND off-loop. + + Two invariants in one, because they were one review finding: the endpoint + must route through :func:`_open_checked_file` instead of hand-rolling a + second spelling of that security boundary, and the whole envelope must + run on a worker thread — an NFS/FUSE-backed document makes realpath/stat/ + open block for seconds, which on the event loop stalls every session's + streaming and the liveness heartbeat. + """ + f = tmp_path / "doc.docx" + _write_docx(str(f), ["content"]) + seen: dict[str, Any] = {} + real_prefix = files_mod._open_checked_file + loop_thread = threading.current_thread() + + def _spy(*args, **kwargs): + seen["thread"] = threading.current_thread() + seen["kwargs"] = kwargs + return real_prefix(*args, **kwargs) + + with ( + patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)), + patch("kiro_crew.dashboard.handlers.files._open_checked_file", side_effect=_spy), + ): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get(f"/api/file-office-preview?path={f}") + assert resp.status == 200 + assert ( + seen.get("thread") is not None + ), "the endpoint must use the shared _open_checked_file prefix, not an inline copy" + assert ( + seen["thread"] is not loop_thread + ), "the open-and-check envelope must not run on the event loop" + # The 50 MB ceiling is now expressed as the prefix's fstat_cap, enforced + # on the fd before zipfile materializes the archive's central directory. + assert seen["kwargs"]["fstat_cap"] == _MAX_UPLOAD_BYTES + + +@pytest.mark.asyncio +async def test_cancellation_is_sel_audited_and_reraised(tmp_path, mock_sel): + """CancelledError during extraction records the access, then propagates.""" + f = tmp_path / "doc.docx" + _write_docx(str(f), ["content"]) + request = make_mocked_request("GET", f"/api/file-office-preview?path={f}") + with ( + patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)), + patch( + "kiro_crew.dashboard.handlers.files.asyncio.to_thread", + side_effect=asyncio.CancelledError(), + ), + ): + with pytest.raises(asyncio.CancelledError): + await api_file_office_preview(request) + cancelled = [ + c + for c in mock_sel.log_tool_invocation.call_args_list + if c.kwargs.get("outcome") == "cancelled" + ] + assert cancelled, "cancelled extraction must still leave an SEL record" + + +@pytest.mark.asyncio +async def test_redaction_runs_before_truncation(tmp_path, mock_sel): + """A credential straddling the cap boundary must not leak as a prefix. + + Redaction must see the FULL extracted text: slicing first would cut the + secret mid-token so the redactor no longer matches it. + """ + f = tmp_path / "creds.docx" + # One paragraph: filler that ends 10 chars before the cap, then a fake + # AKIA credential ID that straddles the boundary. + secret = "AKIAIOSFODNN7EXAMPLE" + filler = "x" * (_OFFICE_PREVIEW_CAP - 10) + _write_docx(str(f), [filler + secret]) + with patch("kiro_crew.dashboard.handlers._validate_dashboard_path", return_value=str(f)): + async with TestClient(TestServer(_make_app())) as client: + resp = await client.get(f"/api/file-office-preview?path={f}") + assert resp.status == 200 + body = await resp.json() + assert body["truncated"] is True + # Neither the full secret nor its cap-cut prefix may appear. + assert secret not in body["text"] + assert secret[:10] not in body["text"] diff --git a/test/windows-collect-ignore.txt b/test/windows-collect-ignore.txt index f27d5c52a42..d9be458cf6e 100644 --- a/test/windows-collect-ignore.txt +++ b/test/windows-collect-ignore.txt @@ -48,3 +48,4 @@ test_file_raw.py # 0o600/0o644 mode-bit assertions test_file_download.py # 0o600/0o644 mode-bit assertions test_dashboard_file_io.py # 0o600/0o644 mode-bit assertions test_dev_fleet_app.py # POSIX app-runner assumptions +test_file_office_preview.py # FILE_READ_SCHEMA rejects C:\ paths (POSIX-only pattern) diff --git a/website/src/components/FileRenderers.tsx b/website/src/components/FileRenderers.tsx index 71df5d68249..a8c38847f61 100644 --- a/website/src/components/FileRenderers.tsx +++ b/website/src/components/FileRenderers.tsx @@ -1,10 +1,11 @@ import { memo, useState, useMemo, useCallback, useEffect, useRef } from 'react' +import { useQuery } from '@tanstack/react-query' import { Download, FileText, Film, Music } from 'lucide-react' import DOMPurify from 'dompurify' import { i18nT } from '../i18n/t' import { ExcalidrawBlock } from './ExcalidrawBlock' -import { fileDownloadUrl, fileStreamUrl } from '../utils/fileReadUrl' +import { fileDownloadUrl, fileStreamUrl, fileOfficePreviewUrl } from '../utils/fileReadUrl' import { useLanguageGeneration } from '../i18n/useLanguageGeneration' /* ── extension helpers ── */ const IMG_EXTS = new Set(['.png', '.jpg', '.jpeg', '.gif', '.bmp', '.webp', '.svg', '.ico']) @@ -289,17 +290,31 @@ export const PdfViewer = memo(function PdfViewer({ filePath }: { filePath: strin ) }) -/* ── Office viewer (download-only card for .docx/.xlsx/.pptx/etc.) ── +/* ── Office viewer ───────────────────────────────────────────────── * * Office binary formats are ZIP archives (OOXML) or legacy OLE compound files * that browsers cannot render inline. Serving them through /api/file-read * decodes them as UTF-8 with errors='replace', producing garbled control-code - * text (raw ZIP bytes starting with 'PK…'). This viewer replaces that broken - * rendering with a filename + extension badge + Download button pointing at - * /api/file-download, which streams the original bytes with attachment - * disposition + nosniff so the file downloads cleanly instead. */ -export const OfficeViewer = memo(function OfficeViewer({ filePath, hideHint }: { filePath: string; hideHint?: boolean }) { - useLanguageGeneration() // memo() bails out of the provider-level repaint; subscribe directly + * text (raw ZIP bytes starting with 'PK…'). + * + * Two rendering states: + * 1. **Preview** — for .docx and .pptx the backend can extract plaintext + * via `kiro_crew.doc_parser.extract_text` (defusedxml-hardened, no + * python-docx / python-pptx dep). We render that text in a scrollable + * pre with a smaller "Download original" button pinned at the bottom. + * 2. **Download-only card** — for extensions the backend can't preview + * (.xls / .xlsx / .doc / .ppt / .odt / .ods / .odp) we render the + * original card: filename + extension badge + full-size Download button. + * This is also the fallback when the preview fetch fails, the document + * is empty, or extract_text returns "" (parse failure). + * + * The preview endpoint returns 415 for unsupported extensions, so anything + * other than a 2xx-with-non-empty-text falls through to the card without + * duplicating the previewable-ext list on the frontend. */ + +/** Card body shared by both rendering states — full-size Download button + * (fallback mode) or compact "Download original" affordance (preview mode). */ +function OfficeCard({ filePath, showBigDownload, hideHint }: { filePath: string; showBigDownload: boolean; hideHint?: boolean }) { // Split on BOTH separators — Kiro Crew ships native on Windows where paths // arrive as `C:\Users\…\report.docx`, and a `/`-only split would surface the // whole path as the "filename". Matches the pattern in MarkdownRenderer.tsx @@ -308,30 +323,111 @@ export const OfficeViewer = memo(function OfficeViewer({ filePath, hideHint }: { const ext = extOf(filePath).replace('.', '').toUpperCase() const url = fileDownloadUrl(filePath) return ( -
-
-
- - +
+
+ + +
+
{filename}
+ {showBigDownload && !hideHint && ( +
+ {i18nT('components.fileRenderers.office_download_hint')}
-
{filename}
- {!hideHint && ( - + ) +} + +type OfficePreviewBody = { text?: string; truncated?: boolean } + +// Extensions the backend can actually extract (mirrors _OFFICE_PREVIEWABLE_EXT +// in dashboard/handlers/files.py). Known-unsupported office formats render the +// download card directly — no fetch, no "Loading preview…" flash for a +// guaranteed 415. The 415 fallback below stays as the safety net if the two +// lists ever drift. +const OFFICE_PREVIEWABLE_EXTS = new Set(['.docx', '.pptx']) + +export const OfficeViewer = memo(function OfficeViewer({ filePath, hideHint }: { filePath: string; hideHint?: boolean }) { + useLanguageGeneration() // memo() bails out of the provider-level repaint; subscribe directly + const filename = filePath.split(/[\\/]/).pop() || filePath + const previewable = OFFICE_PREVIEWABLE_EXTS.has(extOf(filePath)) + // React Query (repo convention for server fetches — see ArtifactPanel / + // AgentSkillsEditor). Keyed on filePath so navigating between .docx files + // in the tree never flashes a stale response; aborts via the provided + // signal on unmount/key change. + const previewQuery = useQuery({ + queryKey: ['office-preview', filePath], + queryFn: async ({ signal }) => { + const res = await fetch(fileOfficePreviewUrl(filePath), { signal }) + if (!res.ok) { + // 415 (unsupported ext), 404, 400, 500 → all fall through to the + // download-only card. We don't distinguish here because a broken + // preview should never block downloading the real file. + return null + } + return await res.json() as OfficePreviewBody + }, + enabled: previewable, + // No staleTime: a reopened file must show its CURRENT contents — the + // document may have been edited since the last preview. Deduping within + // a single mount still applies; only remounts refetch. + staleTime: 0, + retry: false, + }) + + if (previewable && previewQuery.isLoading) { + return ( +
+
+ {i18nT('components.fileRenderers.office_preview_loading')} +
+
+ ) + } + + const body = previewable && !previewQuery.isError ? previewQuery.data : null + if (!body?.text) { + return ( +
+ +
+ ) + } + + // Preview state — scrollable plaintext + compact download affordance at bottom. + // tabIndex + aria-label make the scroll container keyboard-reachable so long + // documents stay readable past the fold without a pointer. + return ( +
+ {/* Keyboard-scrollable region — same pattern as CodeBlock.tsx. */} + {/* eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex */} +
+
{body.text}
+
+
+ {/* Truncation notice lives in the always-visible pinned bar (not after + the 512 KB of text) so users skimming the top of a large document + learn the preview is partial without scrolling to the end. */} + {body.truncated && ( +
+ {i18nT('components.fileRenderers.office_preview_truncated')}
)} - - +
) diff --git a/website/src/i18n/locales/bn.json b/website/src/i18n/locales/bn.json index 8910dca5787..2c5a180e60e 100644 --- a/website/src/i18n/locales/bn.json +++ b/website/src/i18n/locales/bn.json @@ -5746,7 +5746,10 @@ "media_preview_failed": "প্লেব্যাক ব্যর্থ হয়েছে। ব্রাউজার এই ফাইলটি ডিকোড করতে পারছে না।", "more": "আরও", "null": "null", - "office_download_hint": "Office ডকুমেন্ট ইনলাইনে প্রিভিউ করা যায় না। Word, Excel, PowerPoint বা সামঞ্জস্যপূর্ণ কোনও ভিউয়ারে খুলতে ডাউনলোড করুন।", + "office_download_hint": "এই ফাইলের জন্য প্রিভিউ উপলব্ধ নয়। Word, Excel, PowerPoint বা সামঞ্জস্যপূর্ণ ভিউয়ারে খুলতে ডাউনলোড করুন।", + "office_download_original": "মূল ফাইল ডাউনলোড করুন", + "office_preview_loading": "প্রিভিউ লোড হচ্ছে…", + "office_preview_truncated": "প্রিভিউতে শুধুমাত্র নথির শুরুর অংশ দেখানো হয়েছে। সম্পূর্ণ বিষয়বস্তু দেখতে মূল ফাইলটি ডাউনলোড করুন।", "open_in_new_tab": "নতুন ট্যাবে খুলুন", "pdf_preview": "PDF প্রিভিউ", "rows": "সারি", diff --git a/website/src/i18n/locales/de.json b/website/src/i18n/locales/de.json index 82360193202..a6287bf303c 100644 --- a/website/src/i18n/locales/de.json +++ b/website/src/i18n/locales/de.json @@ -5746,7 +5746,10 @@ "media_preview_failed": "Wiedergabe fehlgeschlagen – der Browser kann diese Datei nicht dekodieren.", "more": "mehr", "null": "null", - "office_download_hint": "Office-Dokumente können nicht inline vorangezeigt werden. Zum Öffnen in Word, Excel, PowerPoint oder einem kompatiblen Viewer herunterladen.", + "office_download_hint": "Für diese Datei ist keine Vorschau verfügbar. Laden Sie sie herunter, um sie in Word, Excel, PowerPoint oder einem kompatiblen Programm zu öffnen.", + "office_download_original": "Original herunterladen", + "office_preview_loading": "Vorschau wird geladen …", + "office_preview_truncated": "Die Vorschau zeigt nur den Anfang dieses Dokuments. Laden Sie das Original herunter, um den vollständigen Inhalt zu sehen.", "open_in_new_tab": "In neuem Tab öffnen", "pdf_preview": "PDF-Vorschau", "rows": "Zeilen", diff --git a/website/src/i18n/locales/en-XA.json b/website/src/i18n/locales/en-XA.json index f137bb469f2..257475ac610 100644 --- a/website/src/i18n/locales/en-XA.json +++ b/website/src/i18n/locales/en-XA.json @@ -5581,7 +5581,10 @@ "lines": "[ĺìñèş ········]", "more": "[ɱøŕè ······]", "null": "[ñùĺĺ ······]", - "office_download_hint": "[؃ƒìçè ðøçùɱèñţş çàñ'ţ ƀè þŕèṽìèẁèð ìñĺìñè. Ðøẁñĺøàð ţø øþèñ ìñ Ẁøŕð, Èẋçèĺ, ÞøẁèŕÞøìñţ, øŕ à çøɱþàţìƀĺè ṽìèẁèŕ. ··································]", + "office_download_hint": "[Þŕèṽìèẁ ìşñ'ţ àṽàìĺàƀĺè ƒøŕ ţĥìş ƒìĺè. Ðøẁñĺøàð ţø øþèñ ìñ Ẁøŕð, Èẋçèĺ, ÞøẁèŕÞøìñţ, øŕ à çøɱþàţìƀĺè ṽìèẁèŕ. ································]", + "office_download_original": "[Ðøẁñĺøàð øŕìğìñàĺ ···············]", + "office_preview_loading": "[Ĺøàðìñğ þŕèṽìèẁ… ··············]", + "office_preview_truncated": "[Þŕèṽìèẁ şĥøẁş øñĺý ţĥè ƀèğìññìñğ øƒ ţĥìş ðøçùɱèñţ. Ðøẁñĺøàð ţĥè øŕìğìñàĺ ƒøŕ ţĥè ƒùĺĺ çøñţèñţş. ·····························]", "open_in_new_tab": "[Øþèñ ìñ ñèẁ ţàƀ ··············]", "pdf_preview": "[ÞÐƑ Þŕèṽìèẁ ··········]", "rows": "[ŕøẁş ······]", diff --git a/website/src/i18n/locales/en.json b/website/src/i18n/locales/en.json index b13e67a41f1..8c0959a7e71 100644 --- a/website/src/i18n/locales/en.json +++ b/website/src/i18n/locales/en.json @@ -4229,7 +4229,10 @@ "lines": "lines", "more": "more", "null": "null", - "office_download_hint": "Office documents can't be previewed inline. Download to open in Word, Excel, PowerPoint, or a compatible viewer.", + "office_download_hint": "Preview isn't available for this file. Download to open in Word, Excel, PowerPoint, or a compatible viewer.", + "office_download_original": "Download original", + "office_preview_loading": "Loading preview…", + "office_preview_truncated": "Preview shows only the beginning of this document. Download the original for the full contents.", "open_in_new_tab": "Open in new tab", "pdf_preview": "PDF Preview", "rows": "rows", diff --git a/website/src/i18n/locales/es.json b/website/src/i18n/locales/es.json index a825cb78096..3c141bb3d38 100644 --- a/website/src/i18n/locales/es.json +++ b/website/src/i18n/locales/es.json @@ -5845,7 +5845,10 @@ "media_preview_failed": "Error de reproducción: el navegador no puede decodificar este archivo.", "more": "más", "null": "null", - "office_download_hint": "Los documentos de Office no se pueden previsualizar en línea. Descárgalo para abrirlo en Word, Excel, PowerPoint o un visor compatible.", + "office_download_hint": "La vista previa no está disponible para este archivo. Descárgalo para abrirlo en Word, Excel, PowerPoint o un visor compatible.", + "office_download_original": "Descargar original", + "office_preview_loading": "Cargando la vista previa…", + "office_preview_truncated": "La vista previa muestra solo el principio de este documento. Descarga el original para ver el contenido completo.", "open_in_new_tab": "Abrir en una pestaña nueva", "pdf_preview": "Vista previa PDF", "rows": "filas", diff --git a/website/src/i18n/locales/fr.json b/website/src/i18n/locales/fr.json index 216d4a71e94..28735126baa 100644 --- a/website/src/i18n/locales/fr.json +++ b/website/src/i18n/locales/fr.json @@ -5845,7 +5845,10 @@ "media_preview_failed": "Échec de la lecture : le navigateur ne peut pas décoder ce fichier.", "more": "de plus", "null": "null", - "office_download_hint": "Les documents Office ne peuvent pas être prévisualisés en ligne. Téléchargez-le pour l'ouvrir dans Word, Excel, PowerPoint ou une visionneuse compatible.", + "office_download_hint": "L'aperçu n'est pas disponible pour ce fichier. Téléchargez-le pour l'ouvrir dans Word, Excel, PowerPoint ou un lecteur compatible.", + "office_download_original": "Télécharger l'original", + "office_preview_loading": "Chargement de l'aperçu…", + "office_preview_truncated": "L'aperçu ne montre que le début de ce document. Téléchargez l'original pour voir l'intégralité du contenu.", "open_in_new_tab": "Ouvrir dans un nouvel onglet", "pdf_preview": "Aperçu PDF", "rows": "lignes", diff --git a/website/src/i18n/locales/hi.json b/website/src/i18n/locales/hi.json index 37b81774857..1f6700b5702 100644 --- a/website/src/i18n/locales/hi.json +++ b/website/src/i18n/locales/hi.json @@ -5746,7 +5746,10 @@ "media_preview_failed": "प्लेबैक विफल रहा। ब्राउज़र इस फ़ाइल को डिकोड नहीं कर सकता।", "more": "और", "null": "null", - "office_download_hint": "Office दस्तावेज़ों का इनलाइन पूर्वावलोकन नहीं किया जा सकता। Word, Excel, PowerPoint या किसी संगत व्यूअर में खोलने के लिए डाउनलोड करें।", + "office_download_hint": "इस फ़ाइल के लिए पूर्वावलोकन उपलब्ध नहीं है। Word, Excel, PowerPoint या किसी संगत व्यूअर में खोलने के लिए डाउनलोड करें।", + "office_download_original": "मूल फ़ाइल डाउनलोड करें", + "office_preview_loading": "पूर्वावलोकन लोड हो रहा है…", + "office_preview_truncated": "पूर्वावलोकन में केवल दस्तावेज़ की शुरुआत दिखाई देती है। पूरी सामग्री देखने के लिए मूल फ़ाइल डाउनलोड करें।", "open_in_new_tab": "नए टैब में खोलें", "pdf_preview": "PDF पूर्वावलोकन", "rows": "पंक्तियाँ", diff --git a/website/src/i18n/locales/it.json b/website/src/i18n/locales/it.json index 3d028915e00..e7e04aa4610 100644 --- a/website/src/i18n/locales/it.json +++ b/website/src/i18n/locales/it.json @@ -5845,7 +5845,10 @@ "media_preview_failed": "Riproduzione non riuscita: il browser non riesce a decodificare questo file.", "more": "altri", "null": "null", - "office_download_hint": "I documenti Office non possono essere visualizzati in linea. Scaricalo per aprirlo in Word, Excel, PowerPoint o un visualizzatore compatibile.", + "office_download_hint": "L'anteprima non è disponibile per questo file. Scaricalo per aprirlo in Word, Excel, PowerPoint o un visualizzatore compatibile.", + "office_download_original": "Scarica l'originale", + "office_preview_loading": "Caricamento anteprima…", + "office_preview_truncated": "L'anteprima mostra solo l'inizio di questo documento. Scarica l'originale per vedere il contenuto completo.", "open_in_new_tab": "Apri in una nuova scheda", "pdf_preview": "Anteprima PDF", "rows": "righe", diff --git a/website/src/i18n/locales/ja.json b/website/src/i18n/locales/ja.json index c0a27cb8876..481a1ecfb0c 100644 --- a/website/src/i18n/locales/ja.json +++ b/website/src/i18n/locales/ja.json @@ -5647,7 +5647,10 @@ "media_preview_failed": "再生に失敗しました。ブラウザーはこのファイルをデコードできません。", "more": "その他", "null": "null", - "office_download_hint": "Office ドキュメントはインラインでプレビューできません。Word、Excel、PowerPoint、または互換ビューアーで開くには、ダウンロードしてください。", + "office_download_hint": "このファイルはプレビューできません。ダウンロードして Word、Excel、PowerPoint または互換ビューアで開いてください。", + "office_download_original": "元のファイルをダウンロード", + "office_preview_loading": "プレビューを読み込み中…", + "office_preview_truncated": "プレビューには文書の冒頭のみ表示されています。全文を表示するには元のファイルをダウンロードしてください。", "open_in_new_tab": "新しいタブで開く", "pdf_preview": "PDF プレビュー", "rows": "行", diff --git a/website/src/i18n/locales/ko.json b/website/src/i18n/locales/ko.json index 73b95843db0..3eece8aecd3 100644 --- a/website/src/i18n/locales/ko.json +++ b/website/src/i18n/locales/ko.json @@ -5647,7 +5647,10 @@ "media_preview_failed": "재생에 실패했습니다. 브라우저가 이 파일을 디코딩할 수 없습니다.", "more": "더 보기", "null": "null", - "office_download_hint": "Office 문서는 인라인으로 미리 볼 수 없습니다. Word, Excel, PowerPoint 또는 호환 뷰어에서 열려면 다운로드하세요.", + "office_download_hint": "이 파일은 미리보기를 사용할 수 없습니다. 다운로드하여 Word, Excel, PowerPoint 또는 호환 뷰어에서 여세요.", + "office_download_original": "원본 다운로드", + "office_preview_loading": "미리보기 로드 중…", + "office_preview_truncated": "미리보기는 문서의 시작 부분만 표시합니다. 전체 내용을 보려면 원본을 다운로드하세요.", "open_in_new_tab": "새 탭에서 열기", "pdf_preview": "PDF 미리보기", "rows": "행", diff --git a/website/src/i18n/locales/pt.json b/website/src/i18n/locales/pt.json index 887f1c677c7..91cbd6b0c59 100644 --- a/website/src/i18n/locales/pt.json +++ b/website/src/i18n/locales/pt.json @@ -5845,7 +5845,10 @@ "media_preview_failed": "Falha na reprodução: o navegador não consegue decodificar este arquivo.", "more": "mais", "null": "null", - "office_download_hint": "Documentos do Office não podem ser visualizados diretamente. Baixe para abrir no Word, Excel, PowerPoint ou um visualizador compatível.", + "office_download_hint": "A pré-visualização não está disponível para este arquivo. Baixe-o para abrir no Word, Excel, PowerPoint ou em um visualizador compatível.", + "office_download_original": "Baixar original", + "office_preview_loading": "Carregando visualização…", + "office_preview_truncated": "A pré-visualização mostra apenas o início deste documento. Baixe o original para ver o conteúdo completo.", "open_in_new_tab": "Abrir em nova aba", "pdf_preview": "Pré-visualização de PDF", "rows": "linhas", diff --git a/website/src/i18n/locales/ru.json b/website/src/i18n/locales/ru.json index 4b86d1c1f8d..d6119d43d6f 100644 --- a/website/src/i18n/locales/ru.json +++ b/website/src/i18n/locales/ru.json @@ -5944,7 +5944,10 @@ "media_preview_failed": "Не удалось воспроизвести — браузер не может декодировать этот файл.", "more": "ещё", "null": "null", - "office_download_hint": "Документы Office нельзя просмотреть встроенно. Скачайте, чтобы открыть в Word, Excel, PowerPoint или совместимом просмотрщике.", + "office_download_hint": "Предварительный просмотр недоступен для этого файла. Скачайте его, чтобы открыть в Word, Excel, PowerPoint или совместимой программе.", + "office_download_original": "Скачать оригинал", + "office_preview_loading": "Загрузка предпросмотра…", + "office_preview_truncated": "Предварительный просмотр показывает только начало документа. Скачайте оригинал, чтобы увидеть всё содержимое.", "open_in_new_tab": "Открыть в новой вкладке", "pdf_preview": "Просмотр PDF", "rows": "строк", diff --git a/website/src/i18n/locales/zh-CN.json b/website/src/i18n/locales/zh-CN.json index 0d43ca71847..b25221162e1 100644 --- a/website/src/i18n/locales/zh-CN.json +++ b/website/src/i18n/locales/zh-CN.json @@ -5647,7 +5647,10 @@ "media_preview_failed": "播放失败:浏览器无法解码此文件。", "more": "更多", "null": "null", - "office_download_hint": "无法在线预览 Office 文档。请下载后使用 Word、Excel、PowerPoint 或兼容的查看器打开。", + "office_download_hint": "此文件无法预览。请下载后用 Word、Excel、PowerPoint 或兼容的查看器打开。", + "office_download_original": "下载原文件", + "office_preview_loading": "正在加载预览…", + "office_preview_truncated": "预览仅显示文档开头部分。下载原文件以查看完整内容。", "open_in_new_tab": "在新标签页中打开", "pdf_preview": "PDF 预览", "rows": "行", diff --git a/website/src/test/FileRenderers.test.tsx b/website/src/test/FileRenderers.test.tsx index 6f0e79f3800..de53e5870c8 100644 --- a/website/src/test/FileRenderers.test.tsx +++ b/website/src/test/FileRenderers.test.tsx @@ -1,5 +1,7 @@ -import { describe, it, expect } from 'vitest' -import { fireEvent, render, screen } from '@testing-library/react' +import { describe, it, expect, vi, afterEach } from 'vitest' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import type { ReactNode } from 'react' import { columnLetter, detectFileType, JsonlViewer, OfficeViewer, SheetViewer } from '../components/FileRenderers' describe('detectFileType', () => { @@ -65,28 +67,135 @@ describe('JsonlViewer', () => { }) }) +/** OfficeViewer (and SheetViewer's fallback card, which renders it) fetch via + * React Query, so renders need a QueryClientProvider. Fresh client per render + * keeps the per-filePath query cache from leaking between tests; retry + * disabled so error paths settle in one pass. */ +function renderWithQuery(ui: ReactNode) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }) + return render({ui}) +} + describe('OfficeViewer', () => { - it('renders filename, extension badge, and a Download link pointing at /api/file-download', () => { - render() - // Filename shown to the user (basename, not full path). - expect(screen.getByText('quarterly-report.docx')).toBeInTheDocument() - // Extension badge — uppercase, drives the visual "this is a DOCX" cue. - expect(screen.getByText('DOCX')).toBeInTheDocument() - // Accessible download control routed through /api/file-download so the - // browser sees attachment disposition + nosniff and downloads raw bytes - // instead of trying to render UTF-8-decoded ZIP garbage. - const link = screen.getByRole('link', { name: /quarterly-report\.docx/i }) - expect(link).toHaveAttribute('href', expect.stringContaining('/api/file-download?path=')) - expect(link).toHaveAttribute('href', expect.stringContaining('quarterly-report.docx')) - expect(link).toHaveAttribute('download', 'quarterly-report.docx') + const realFetch = globalThis.fetch + + + /** Stub /api/file-office-preview with a Response-shaped object. Mirrors the + * pattern used in MarkdownRenderer.test.tsx for the file-read HEAD probe. */ + function stubPreview(body: { text?: string; truncated?: boolean; error?: string } | null, ok = true, status = 200) { + globalThis.fetch = vi.fn(() => + Promise.resolve({ + ok, + status, + json: () => Promise.resolve(body ?? {}), + } as unknown as Response), + ) as unknown as typeof fetch + } + + afterEach(() => { + globalThis.fetch = realFetch + vi.restoreAllMocks() + }) + + it('renders the plaintext preview when /api/file-office-preview returns text', async () => { + // The component's decision about which UI state to render is driven + // entirely by (ok, body.text) — matches the backend contract + // in `api_file_office_preview`. + stubPreview({ + text: 'Introduction\n\nThis is the first paragraph of the document.', + truncated: false, + }) + renderWithQuery() + await waitFor(() => { + expect(screen.getByText(/Introduction/)).toBeInTheDocument() + }) + expect(screen.getByText(/first paragraph/)).toBeInTheDocument() + // Compact "Download original" affordance is present beneath the preview, + // not the full-size button — this is the preview state. + expect(screen.getByRole('link', { name: /quarterly-report\.docx/i })).toBeInTheDocument() + expect(screen.getByText('Download original')).toBeInTheDocument() }) - it('extracts the basename from a Windows path with backslash separators', () => { + it('makes the preview scroll container keyboard-focusable', async () => { + // Long documents must stay readable past the fold without a pointer — + // the scroll region carries tabIndex=0 and an accessible name. + stubPreview({ text: 'Some document text', truncated: false }) + renderWithQuery() + await waitFor(() => { + expect(screen.getByText('Some document text')).toBeInTheDocument() + }) + const region = screen.getByRole('region', { name: 'quarterly-report.docx' }) + expect(region).toHaveAttribute('tabindex', '0') + }) + + it('falls back to the download card when /api/file-office-preview returns 415', async () => { + // Server-side safety net: if the backend rejects a nominally previewable + // extension (list drift, direct API), the component MUST render the + // full-size download card — never block the user from getting the file. + stubPreview({ error: 'unsupported format for inline preview' }, false, 415) + renderWithQuery() + await waitFor(() => { + expect(screen.getByText('report.docx')).toBeInTheDocument() + }) + expect(screen.getByText('Download')).toBeInTheDocument() + expect(screen.getByText(/Preview isn't available for this file/i)).toBeInTheDocument() + }) + + it('renders the download card for never-previewable extensions without fetching', async () => { + // Known-unsupported formats (.xls/.doc/.odt…) short-circuit client-side: + // no fetch, no "Loading preview…" flash for a guaranteed 415. + const fetchSpy = vi.fn() + globalThis.fetch = fetchSpy as unknown as typeof fetch + renderWithQuery() + await waitFor(() => { + expect(screen.getByText('legacy.xls')).toBeInTheDocument() + }) + expect(screen.getByText('Download')).toBeInTheDocument() + expect(fetchSpy).not.toHaveBeenCalled() + }) + + it('falls back to the download card when the fetch itself throws', async () => { + globalThis.fetch = vi.fn(() => Promise.reject(new TypeError('Failed to fetch'))) as unknown as typeof fetch + renderWithQuery() + await waitFor(() => { + expect(screen.getByText('quarterly-report.docx')).toBeInTheDocument() + }) + expect(screen.getByText('Download')).toBeInTheDocument() + }) + + it('falls back to the download card when extraction returns empty text', async () => { + // doc_parser returns "" for both a blank document and a parse failure — + // the frontend treats empty text as "no preview" and shows the card. + stubPreview({ text: '', truncated: false }) + renderWithQuery() + await waitFor(() => { + expect(screen.getByText('blank.docx')).toBeInTheDocument() + }) + expect(screen.getByText('Download')).toBeInTheDocument() + }) + + it('renders the truncation notice in the pinned footer when the backend flags truncation', async () => { + stubPreview({ + text: 'A very long document that would keep going...', + truncated: true, + }) + renderWithQuery() + await waitFor(() => { + expect(screen.getByText(/Preview shows only the beginning/i)).toBeInTheDocument() + }) + }) + + it('extracts the basename from a Windows path with backslash separators', async () => { // Kiro Crew ships native on Windows where filePath arrives as // C:\Users\...\report.docx. A `/`-only split would surface the whole // path — split on BOTH separators to match MarkdownRenderer/VectorMemoryCard. - render() - expect(screen.getByText('report.docx')).toBeInTheDocument() + stubPreview({}, false, 415) // force fallback so the download card is visible + renderWithQuery() + await waitFor(() => { + expect(screen.getByText('report.docx')).toBeInTheDocument() + }) expect(screen.queryByText(/C:\\Users/)).not.toBeInTheDocument() }) }) @@ -160,7 +269,8 @@ describe('SheetViewer', () => { // 422 = parse failure; the viewer must never be worse than the card it replaced, // and the banner must not claim xlsx can never preview inline. stubFetch(async () => ({ ok: false, status: 422, json: async () => ({ error: 'cannot parse workbook' }) })) - render() + // Fallback card renders OfficeViewer, which calls useQuery — needs the provider. + renderWithQuery() expect(await screen.findByText('model.xlsx')).toBeInTheDocument() expect(screen.getByText(/Preview failed/)).toBeInTheDocument() const link = screen.getByRole('link', { name: /model\.xlsx/i }) @@ -169,7 +279,7 @@ describe('SheetViewer', () => { it('degrades to the download card when fetch itself rejects', async () => { stubFetch(async () => { throw new Error('network down') }) - render() + renderWithQuery() expect(await screen.findByRole('link', { name: /model\.xlsx/i })).toBeInTheDocument() }) diff --git a/website/src/utils/fileReadUrl.ts b/website/src/utils/fileReadUrl.ts index fc0def68412..0c0a9f27b1e 100644 --- a/website/src/utils/fileReadUrl.ts +++ b/website/src/utils/fileReadUrl.ts @@ -36,3 +36,21 @@ export function fileDownloadUrl(filePath: string): string { export function fileStreamUrl(filePath: string): string { return withResolve('/api/file-stream?path=' + encodeURIComponent(filePath), filePath) } + +/** Build the /api/file-office-preview URL — extracts plaintext from a + * .docx / .pptx for inline preview in the file viewer. + * + * The backend uses `kiro_crew.doc_parser.extract_text` (defusedxml-hardened + * ZIP+XML parser, no python-docx / python-pptx dep). Returns 415 when the + * extension isn't previewable (.xls/.xlsx/.doc/.ppt/ODF) so the caller can + * fall back to the download card. See `api_file_office_preview` in + * `src/kiro_crew/dashboard/handlers/files.py`. + * + * Derived from fileDownloadUrl rather than restated: the two endpoints take + * the identical query shape (path + optional resolve=1), so swapping the + * endpoint segment keeps one owner for the construction. The swap cannot + * collide with the encoded path value — encodeURIComponent turns its + * slashes into %2F, so the raw endpoint string appears exactly once. */ +export function fileOfficePreviewUrl(filePath: string): string { + return fileDownloadUrl(filePath).replace('/api/file-download', '/api/file-office-preview') +}