fix(uploads): report rejected filenames and make success mean "a file was stored" (#7165)

Unsafe filenames and unnamed parts were dropped silently: the response kept
`success: true` with an empty `files` list, so the #7122 frontend gate never
blocked a chat send whose attachment had been rejected (#7160).

Both drop arms now record the rejected name in `skipped_files`, and `success`
follows `bool(uploaded_files)`: an all-rejected batch fails, a mixed batch
succeeds and lists what was dropped.

backend/app/gateway/routers/uploads.py

@@ -441,11 +441,22 @@ async def upload_files(
             raise HTTPException(status_code=400, detail=str(e))
         for file in files:
             if not file.filename:
+                # Starlette builds an UploadFile even for a part with an empty
+                # filename; dropping it silently kept `success: true` over an
+                # empty `files` + `skipped_files`, bypassing the #7122 gate
+                # exactly like the unsafe-name arm below (#7160).
+                logger.warning("Skipping upload part without a filename")
+                skipped_files.append("(unnamed file)")
                 continue
             try:
                 file_info = await service.ingest_chunks(_stream_upload_file(file), display_name=file.filename)
-            except UnsafeFilenameError:
-                logger.warning(f"Skipping file with unsafe filename: {file.filename!r}")
+            except UnsafeFilenameError as exc:
+                # The rejected name must reach the caller: leaving it out of
+                # skipped_files kept `success: true` over an empty `files`
+                # list, which let the chat send proceed without the
+                # attachment the user asked for (#7160).
+                logger.warning("Skipping upload with unsafe filename %r: %s", file.filename, exc)
+                skipped_files.append(file.filename)
                 continue
             except UnsafeUploadDestinationError as e:
                 logger.warning("Skipping upload with unsafe destination %s: %s", file.filename, e)
@@ -467,7 +478,11 @@ async def upload_files(
             message += f"; skipped {len(skipped_files)} unsafe file(s)"
 
         return UploadResponse(
-            success=not skipped_files,
+            # ``success`` means "at least one file was stored": an all-rejected
+            # batch must be a failure so the #7122 frontend gate blocks the
+            # send, while a mixed batch still succeeds and reports the dropped
+            # names through ``skipped_files`` (#7160).
+            success=bool(uploaded_files),
             files=uploaded_files,
             message=message,
             skipped_files=skipped_files,

backend/tests/blocking_io/test_uploads_router.py

@@ -92,6 +92,77 @@ async def test_upload_endpoint_mounted_provider_does_not_block_event_loop(tmp_pa
     assert await asyncio.to_thread(target.read_bytes) == b"hello uploads"
 
 
+async def test_unsafe_filename_is_reported_as_skipped(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    """A rejected filename must reach the caller in ``skipped_files`` (#7160)."""
+    _reset_paths(tmp_path, monkeypatch)
+    monkeypatch.setattr(uploads, "get_sandbox_provider", lambda: _MountedProvider())
+
+    result = await call_unwrapped(
+        uploads.upload_files,
+        "t-unsafe-mixed",
+        request=None,
+        files=[
+            UploadFile(filename="ok.txt", file=BytesIO(b"kept document")),
+            UploadFile(filename="bad\x00name.txt", file=BytesIO(b"dropped document")),
+        ],
+        config=SimpleNamespace(),
+    )
+
+    # A mixed batch still succeeds because a file was stored; the dropped name
+    # is visible in skipped_files instead of disappearing.
+    assert result.success is True
+    assert [item.filename for item in result.files] == ["ok.txt"]
+    assert result.skipped_files == ["bad\x00name.txt"]
+    assert "skipped 1 unsafe file(s)" in result.message
+    user_id = get_effective_user_id()
+    listing = await asyncio.to_thread(lambda: sorted(path.name for path in get_uploads_dir("t-unsafe-mixed", user_id=user_id).iterdir()))
+    assert listing == ["ok.txt"]
+
+
+async def test_all_unsafe_filenames_report_failure(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    """Every file rejected means ``success: false``: the #7122 frontend gate
+    blocks the chat send on that, so a silent ``success: true`` here let a
+    message go out without any of the requested attachments."""
+    _reset_paths(tmp_path, monkeypatch)
+    monkeypatch.setattr(uploads, "get_sandbox_provider", lambda: _MountedProvider())
+
+    result = await call_unwrapped(
+        uploads.upload_files,
+        "t-unsafe-all",
+        request=None,
+        files=[
+            UploadFile(filename="bad\x00one.txt", file=BytesIO(b"dropped one")),
+            UploadFile(filename="bad\x00two.txt", file=BytesIO(b"dropped two")),
+        ],
+        config=SimpleNamespace(),
+    )
+
+    assert result.success is False
+    assert result.files == []
+    assert result.skipped_files == ["bad\x00one.txt", "bad\x00two.txt"]
+
+
+async def test_unnamed_part_is_reported_as_skipped(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    """A multipart part with an empty filename must not vanish silently (#7160)."""
+    _reset_paths(tmp_path, monkeypatch)
+    monkeypatch.setattr(uploads, "get_sandbox_provider", lambda: _MountedProvider())
+
+    result = await call_unwrapped(
+        uploads.upload_files,
+        "t-unnamed-part",
+        request=None,
+        files=[
+            UploadFile(filename="", file=BytesIO(b"dropped part")),
+            UploadFile(filename="ok.txt", file=BytesIO(b"kept document")),
+        ],
+        config=SimpleNamespace(),
+    )
+
+    assert result.success is True
+    assert [item.filename for item in result.files] == ["ok.txt"]
+    assert result.skipped_files == ["(unnamed file)"]
+
+
 @pytest.mark.parametrize(
     "filename", [".upload-notes.part", r"folder\.upload-notes.part", r"C:\users\.upload-notes.part", ".upload-notes.part.", ".upload-notes.part ", r"folder\.upload-notes.part. ", ".UPLOAD-NOTES.PART", r"folder\.Upload-NoTeS.Part ."]
 )

backend/tests/test_uploads_router.py

@@ -1260,8 +1260,12 @@ def test_upload_files_rejects_dotdot_and_dot_filenames(tmp_path):
         for bad_name in ["..", "."]:
             file = UploadFile(filename=bad_name, file=BytesIO(b"data"))
             result = asyncio.run(call_unwrapped(uploads.upload_files, "thread-local", request=MagicMock(), files=[file], config=SimpleNamespace()))
-            assert result.success is True
+            # Nothing was stored, so the batch must not report success: the
+            # #7122 frontend gate keys on it, and a silent success let a chat
+            # send proceed without the attachment (#7160).
+            assert result.success is False
             assert result.files == [], f"Expected no files for unsafe filename {bad_name!r}"
+            assert result.skipped_files == [bad_name]
 
         # Path-traversal prefixes are stripped to the basename and accepted safely
         file = UploadFile(filename="../etc/passwd", file=BytesIO(b"data"))