fix(office): stop all of LibreOffice at its time limit — kill the soffice process group

.github/workflows/docker-pr.yml

@@ -436,3 +436,51 @@ jobs:
           assert out.read_bytes()[:4] == b"%PDF", "no PDF produced"
           print("ok: gs stub shadows the real gs for soffice; engine=%s" % conv.engine_used)
           PY
+
+      # soffice only launches LibreOffice: the conversion runs in a child of
+      # the launcher (soffice.bin). Past the time limit the DOCX converter
+      # kills the whole process group (app/converters/document.py::_run_soffice).
+      # Verify in the real image, with a document too long to finish in time,
+      # that the worker ran mid-conversion and is gone after the limit.
+      - name: Check a LibreOffice timeout stops all of LibreOffice
+        if: matrix.target == 'office'
+        run: |
+          docker exec -i filemorph python - <<'PY'
+          import subprocess, tempfile, threading, time
+          from pathlib import Path
+          from docx import Document
+          from app.converters.document import _convert_via_libreoffice
+
+          def workers():
+              """Running soffice.bin processes: (pid, parent pid, process group)."""
+              found = []
+              for stat in Path("/proc").glob("[0-9]*/stat"):
+                  try:
+                      text = stat.read_text()
+                  except OSError:
+                      continue
+                  state, ppid, pgid = text[text.rindex(")") + 2 :].split()[:3]
+                  if text[text.index("(") + 1 : text.rindex(")")] == "soffice.bin" and state != "Z":
+                      found.append((int(stat.parent.name), int(ppid), int(pgid)))
+              return found
+
+          work = Path(tempfile.mkdtemp())
+          doc = Document()
+          for _ in range(10000):
+              doc.add_paragraph("FileMorph time limit probe. " * 16)
+          doc.save(work / "long.docx")
+          mid = []
+          threading.Timer(2, lambda: mid.extend(workers())).start()
+          try:
+              _convert_via_libreoffice(work / "long.docx", work / "out.pdf", timeout_s=5)
+          except subprocess.TimeoutExpired:
+              pass
+          else:
+              raise SystemExit("LibreOffice finished within 5 s; the check needs a longer document")
+          assert mid, "no LibreOffice worker ran mid-conversion; the check proves nothing"
+          deadline = time.monotonic() + 5
+          while workers() and time.monotonic() < deadline:
+              time.sleep(0.1)
+          assert not workers(), f"LibreOffice still running after its time limit: {workers()}"
+          print(f"ok: worker {mid} (pid, parent pid, group) stopped at the time limit")
+          PY

app/converters/document.py

@@ -6,6 +6,7 @@
 import os
 import re
 import shutil
+import signal
 import subprocess
 import zipfile
 from pathlib import Path
@@ -238,6 +239,37 @@ def _soffice_child_env(shim_dir: Path, scratch_dir: Path) -> dict[str, str]:
     return env
 
 
+def _run_soffice(
+    args: list[str], *, timeout: float, env: dict[str, str]
+) -> subprocess.CompletedProcess:
+    """``subprocess.run(args, capture_output=True, timeout=timeout, env=env)``
+    for ``soffice``, except that past ``timeout`` all of LibreOffice stops.
+
+    ``soffice`` is a launcher: the conversion runs in LibreOffice's worker
+    (``soffice.bin``), a child of the launcher. Past its timeout,
+    ``subprocess.run`` stops only the program it started, which would leave
+    the worker running. Started in a session of its own, the call is one
+    process group, and past the timeout — or on any other error while
+    waiting — the whole group is killed.
+    """
+    with subprocess.Popen(  # noqa: S603 — binary resolved via shutil.which, no shell
+        args,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        env=env,
+        start_new_session=True,
+    ) as proc:
+        try:
+            stdout, stderr = proc.communicate(timeout=timeout)
+        except BaseException:
+            if os.name == "posix":
+                os.killpg(proc.pid, signal.SIGKILL)
+            else:  # pragma: no cover - os.killpg is POSIX-only
+                proc.kill()
+            raise
+    return subprocess.CompletedProcess(args, proc.returncode, stdout, stderr)
+
+
 def _convert_via_libreoffice(input_path: Path, output_path: Path, timeout_s: int) -> None:
     """Render DOCX → PDF using LibreOffice headless.
 
@@ -265,7 +297,7 @@ def _convert_via_libreoffice(input_path: Path, output_path: Path, timeout_s: int
         # parallel instances. The user-profile directory is per-call so two
         # concurrent conversions don't clobber each other's lockfiles.
         profile_uri = (work_dir / "userprofile").resolve().as_uri()
-        result = subprocess.run(  # noqa: S603 — binary resolved via shutil.which, no shell
+        result = _run_soffice(
             [
                 binary,
                 "--headless",
@@ -279,9 +311,7 @@ def _convert_via_libreoffice(input_path: Path, output_path: Path, timeout_s: int
                 str(work_dir),
                 str(input_path),
             ],
-            capture_output=True,
             timeout=timeout_s,
-            check=False,
             # Reduced child env: app secrets dropped, gs stubs ahead of the
             # real Ghostscript on PATH. See _soffice_child_env.
             env=_soffice_child_env(shim_dir, scratch_dir),

changelog.d/2026-10-29-libreoffice-timeout.md

@@ -0,0 +1,7 @@
+### Fixed — a LibreOffice time limit stops all of LibreOffice
+
+When a DOCX → PDF conversion through LibreOffice (office image) — including a
+Word file inserted into a PDF — reaches `OFFICE_SUBPROCESS_TIMEOUT_SECONDS`,
+the converter now stops LibreOffice's worker process (`soffice.bin`) together
+with the `soffice` launcher, so the conversion ends completely at the limit.
+For this, `soffice` runs in a process group of its own.

docs/architecture.md

@@ -96,7 +96,9 @@ Every conversion or compression request follows the same path:
    `MEDIA_SUBPROCESS_TIMEOUT_SECONDS` (default 85 s for all of a request's
    audio and video work; video → GIF at most 85 s for all its steps
    together), LibreOffice `OFFICE_SUBPROCESS_TIMEOUT_SECONDS` (default
-   60 s), Ghostscript 60 s. On Linux ffmpeg and ffprobe start through
+   60 s), Ghostscript 60 s. On Linux LibreOffice runs in a process group
+   of its own, so its limit stops the worker (`soffice.bin`) along with
+   the `soffice` launcher. On Linux ffmpeg and ffprobe start through
    `app/converters/_limited_exec.py`, capped at 3 GiB of address space and
    at the request's output cap per file written.
    PDF → TXT runs pypdf in a child Python process

docs/security-overview.md

@@ -300,14 +300,20 @@ keeps working, as does the pure-Python (mammoth) path. This is in the same
 spirit as the Pillow setting that keeps WeasyPrint from reaching Ghostscript
 (`app/core/image_hardening.py`).
 
+Past its time limit (`OFFICE_SUBPROCESS_TIMEOUT_SECONDS`), the subprocess is
+stopped as a whole: `soffice` starts in a process group of its own, so
+LibreOffice's worker process (`soffice.bin`) is killed along with the launcher
+(`_run_soffice` in `app/converters/document.py`).
+
 The PDF/A re-render runs only on an upload whose first non-blank bytes are the
 `%PDF-` header (`_has_pdf_header` in `app/converters/pdfa.py`); any other upload
 is refused with a caller-safe `400` before that stage, so only files that
 present as PDFs reach Ghostscript.
 
-`tests/test_office_gs_isolation.py` pins the subprocess environment, and
-`.github/workflows/docker-pr.yml` exercises a DOCX→PDF conversion on the office
-image.
+`tests/test_office_gs_isolation.py` pins the subprocess environment,
+`tests/test_office_timeout.py` the stop at the time limit, and
+`.github/workflows/docker-pr.yml` exercises a DOCX→PDF conversion and a time
+limit on the office image.
 
 ### Decompression bombs
 

docs/self-hosting.md

@@ -159,9 +159,10 @@ fallback — it makes a missing `soffice` fail loud instead of silently
 degrading).
 
 `OFFICE_SUBPROCESS_TIMEOUT_SECONDS` (default `60`) bounds how long a
-single `soffice --convert-to` call may run before it is killed —
-raise it if you regularly convert long, complex Word documents on a
-slower host. Ignored when `FILEMORPH_OFFICE_ENGINE=mammoth`.
+single `soffice --convert-to` call may run before it is killed, together
+with LibreOffice's worker process (`soffice.bin`) — raise it if you
+regularly convert long, complex Word documents on a slower host. Ignored
+when `FILEMORPH_OFFICE_ENGINE=mammoth`.
 
 ### Verifying signatures
 

tests/test_log_privacy.py

@@ -443,6 +443,7 @@ def test_tool_errors_quote_stderr_only_with_app_debug(monkeypatch, tmp_path, deb
 
     monkeypatch.setattr(settings, "app_debug", debug)
     monkeypatch.setattr(subprocess, "run", _failed_run)
+    monkeypatch.setattr(document, "_run_soffice", _failed_run)
     monkeypatch.setattr(shutil, "which", lambda name: f"/usr/bin/{name}")
     monkeypatch.setattr(_ghostscript, "_GS_BINARY", "gs")
     errors = []
@@ -459,8 +460,8 @@ def test_tool_errors_quote_stderr_only_with_app_debug(monkeypatch, tmp_path, deb
     assert all("exit" in error or "status" in error for error in errors), errors
     # soffice exits 0 but writes no PDF: the same rule for its stderr.
     monkeypatch.setattr(
-        subprocess,
-        "run",
+        document,
+        "_run_soffice",
         lambda *a, **k: subprocess.CompletedProcess(a, 0, b"", b"SECRET-METADATA"),
     )
     with pytest.raises(RuntimeError) as excinfo:

tests/test_office_gs_isolation.py

@@ -9,8 +9,9 @@
 keep working, with the app's secrets dropped from that gs as well; the
 pure-Python (mammoth) path is unaffected.
 
-These tests fake ``shutil.which``/``subprocess.run`` so the wiring is asserted
-on any platform, with no real ``soffice`` or ``gs`` installed. The real office
+These tests fake ``shutil.which`` and the calls that start ``soffice``/``gs``
+(``_run_soffice``, ``subprocess.run``) so the wiring is asserted on any
+platform, with no real ``soffice`` or ``gs`` installed. The real office
 image is exercised end-to-end in ``.github/workflows/docker-pr.yml``.
 """
 
@@ -20,6 +21,7 @@
 import shutil
 import stat
 import subprocess
+import sys
 from pathlib import Path
 
 from app.converters import _ghostscript, document
@@ -95,8 +97,8 @@ def test_child_env_prepends_shims_and_drops_secrets(monkeypatch, tmp_path):
 
 def test_convert_via_libreoffice_runs_soffice_with_hardened_env(monkeypatch, tmp_path):
     """The routing guard the hardening turns on: ``soffice`` is invoked with a
-    Ghostscript stub ahead of any real gs, app secrets stripped, documented
-    flags, and no shell."""
+    Ghostscript stub ahead of any real gs, app secrets stripped, and
+    documented flags."""
     real_gs_dir = str(tmp_path / "usr-bin")
     fake_soffice = str(tmp_path / "soffice")
     monkeypatch.setenv("PATH", os.pathsep.join([str(tmp_path / "venv"), real_gs_dir]))
@@ -127,7 +129,7 @@ def fake_run(argv, **kwargs):
         (outdir / f"{Path(argv[-1]).stem}.pdf").write_bytes(b"%PDF-1.7\n")
         return subprocess.CompletedProcess(argv, 0, stdout=b"", stderr=b"")
 
-    monkeypatch.setattr(document.subprocess, "run", fake_run)
+    monkeypatch.setattr(document, "_run_soffice", fake_run)
 
     src = tmp_path / "in.docx"
     src.write_bytes(b"PK\x03\x04")  # the fake runner never reads it
@@ -143,10 +145,7 @@ def fake_run(argv, **kwargs):
     assert "pdf:writer_pdf_Export" in argv
 
     kwargs = calls["kwargs"]
-    assert kwargs["check"] is False
-    assert kwargs["capture_output"] is True
     assert kwargs["timeout"] == 42
-    assert kwargs.get("shell", False) is False  # run() defaults to no shell
 
     env = kwargs["env"]
     assert "STRIPE_SECRET_KEY" not in env  # secret stripped from the child
@@ -157,6 +156,16 @@ def fake_run(argv, **kwargs):
     assert os.environ["STRIPE_SECRET_KEY"] == "sk_live_do_not_leak"
 
 
+def test_run_soffice_hands_over_the_env_and_returns_like_subprocess_run():
+    """``_run_soffice`` starts the program with exactly the env it is given and
+    returns its status and captured output (soffice's stderr can quote the
+    document) the way ``subprocess.run(capture_output=True)`` would."""
+    code = "import os, sys; sys.stdout.write(os.environ['FM_PROBE']); sys.stderr.write('e'); sys.exit(3)"
+    env = {**os.environ, "FM_PROBE": "env reached the child"}
+    result = document._run_soffice([sys.executable, "-c", code], timeout=30, env=env)
+    assert (result.returncode, result.stdout, result.stderr) == (3, b"env reached the child", b"e")
+
+
 def test_the_pdfa_rerender_runs_ghostscript_without_secrets(monkeypatch, tmp_path):
     """FileMorph's own Ghostscript call gets the app's environment without its
     secrets too, with ``PATH`` as it is: no stub there, it runs the real gs."""

tests/test_office_timeout.py

@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""Past its time limit, a LibreOffice conversion stops all of LibreOffice.
+
+``soffice`` is a launcher; the conversion runs in a child of it
+(``soffice.bin``). A stand-in launcher of the same shape — a shell script that
+starts a worker and waits for it — shows that stopping only the launcher, as
+``subprocess.run`` does past its timeout, leaves the worker running, and that
+``_convert_via_libreoffice`` stops the worker as well. The real LibreOffice is
+checked the same way in ``.github/workflows/docker-pr.yml``.
+"""
+
+from __future__ import annotations
+
+import os
+import signal
+import subprocess
+import sys
+import time
+from pathlib import Path
+
+import pytest
+
+from app.converters import document
+
+pytestmark = pytest.mark.skipif(sys.platform != "linux", reason="reads processes from /proc")
+
+
+def _stand_in_soffice(tmp_path: Path) -> tuple[Path, Path]:
+    """A launcher that starts a worker, writes the worker's PID to a file and
+    waits for it. Returns the launcher and the PID file."""
+    pid_file = tmp_path / "worker.pid"
+    launcher = tmp_path / "soffice"
+    launcher.write_text(
+        f'#!/bin/sh\nsleep 60 &\necho $! > "{pid_file}"\nwait\n', encoding="utf-8", newline="\n"
+    )
+    launcher.chmod(0o755)
+    return launcher, pid_file
+
+
+def _running(pid: int) -> bool:
+    """Whether ``pid`` runs; a process that ended but is not reaped yet counts as ended."""
+    try:
+        stat = Path(f"/proc/{pid}/stat").read_text()
+    except OSError:
+        return False
+    return stat.rsplit(")", 1)[1].split()[0] != "Z"
+
+
+def test_a_plain_timeout_leaves_the_worker_running(tmp_path):
+    # The case the fix is for, and proof that the stand-in shows it: without
+    # this, the next test would also pass with a worker that ends on its own.
+    launcher, pid_file = _stand_in_soffice(tmp_path)
+    with pytest.raises(subprocess.TimeoutExpired):
+        subprocess.run([str(launcher)], capture_output=True, timeout=1, check=False)
+    worker = int(pid_file.read_text())
+    alive = _running(worker)
+    if alive:
+        os.kill(worker, signal.SIGKILL)
+    assert alive
+
+
+def test_a_libreoffice_timeout_stops_the_worker_too(monkeypatch, tmp_path):
+    launcher, pid_file = _stand_in_soffice(tmp_path)
+    monkeypatch.setattr(document.shutil, "which", lambda name: str(launcher))
+    src = tmp_path / "in.docx"
+    src.write_bytes(b"PK\x03\x04")  # the stand-in never reads it
+    with pytest.raises(subprocess.TimeoutExpired):
+        document._convert_via_libreoffice(src, tmp_path / "out.pdf", timeout_s=1)
+    worker = int(pid_file.read_text())
+    deadline = time.monotonic() + 5
+    while _running(worker) and time.monotonic() < deadline:
+        time.sleep(0.05)
+    alive = _running(worker)
+    if alive:
+        os.kill(worker, signal.SIGKILL)  # not left behind when the test fails
+    assert not alive