🛡️ Sentinel: [HIGH] 임시 파일 정리 누락으로 인한 DoS 취약점 수정

.jules/sentinel.md

@@ -42,6 +42,11 @@
 **Learning:** For large file uploads, loading the entire payload into a single Python object (even just to process or save it) creates a bottleneck where large chunks of contiguous memory are required simultaneously. The Strix security scanner will flag this as a Resource Exhaustion Vulnerability ("security theater") if you attempt to just bound a single `file.read()`.
 **Prevention:** Stream the chunks (e.g. 8192 bytes) directly to a `NamedTemporaryFile` on disk while verifying the accumulation does not exceed the maximum allowed payload size. Ensure the temporary file is securely unlinked in a `finally` block or when an upload limit exception is raised.
 
+## 2026-07-09 - Keep upload cleanup non-fatal and observable
+**Vulnerability:** Temporary-file cleanup can fail after a successful parse because of filesystem races, antivirus locks, or platform-specific deletion semantics. If cleanup exceptions are allowed to propagate, a successful parse can become a 500 while still leaving unclear forensic evidence.
+**Learning:** Cleanup must be guaranteed on all upload paths, but cleanup failure handling should be isolated from the user-facing parse result and logged with enough context for operators to see why disk hygiene failed.
+**Prevention:** Run upload temporary-file unlinking in the endpoint `finally` block, catch `OSError`, and log the temporary path at exception level without exposing it in public API responses.
+
 ## 2025-02-28 - [Subprocess argument injection via newlines]
  **Vulnerability:** Unsanitized user inputs containing newline (`\n`) and carriage return (`\r`) characters passed as arguments to subprocesses can lead to command and log injection vulnerabilities, even when `shell=False` is used, depending on how downstream CLI tools process the inputs.
  **Learning:** Standard shell metacharacter filters (like `[\0&;|`$<>]`) are insufficient to prevent injection if they omit whitespace control characters. Attackers can inject newlines to manipulate tool behavior or spoof log entries if the downstream executable processes inputs line-by-line or uses them in script evaluation.

AGENTS.md

@@ -184,4 +184,3 @@ follow it.
   (e.g. layout detection, reading-order recovery, table structure
   recognition).
 <!-- END cwl-agent-guidance -->
-

src/newsdom_api/main.py

@@ -156,7 +156,7 @@ async def parse(
     if file_size is not None and file_size > MAX_PARSE_UPLOAD_BYTES:
         raise HTTPException(status_code=413, detail=PAYLOAD_TOO_LARGE_DETAIL)
 
-    tmp_path = None
+    tmp_path: Path | None = None
     try:
         header = await file.read(5)
         if header != b"%PDF-":
@@ -167,17 +167,22 @@ async def parse(
 
         with tempfile.NamedTemporaryFile(delete=False) as tmp:
             tmp_path = Path(tmp.name)
+            LOGGER.debug("Created temporary upload file %s", tmp_path)
             tmp.write(header)
 
             bytes_read = len(header)
             while chunk := await file.read(8192):
                 bytes_read += len(chunk)
                 if bytes_read > MAX_PARSE_UPLOAD_BYTES:
+                    LOGGER.warning(
+                        "Rejecting upload over limit: %s bytes read", bytes_read
+                    )
                     raise HTTPException(
                         status_code=413, detail=PAYLOAD_TOO_LARGE_DETAIL
                     )
                 tmp.write(chunk)
 
+        LOGGER.debug("Wrote %s upload bytes to %s", bytes_read, tmp_path)
         _validate_pdf_structure(tmp_path)
         return await asyncio.to_thread(
             parse_pdf, tmp_path, filename=file.filename or "upload.pdf"
@@ -188,5 +193,8 @@ async def parse(
     except MineruIncompleteOutputError:
         raise HTTPException(status_code=502, detail="Bad Gateway") from None
     finally:
-        if tmp_path and tmp_path.exists():
-            tmp_path.unlink(missing_ok=True)
+        if tmp_path is not None:
+            try:
+                tmp_path.unlink(missing_ok=True)
+            except OSError:
+                LOGGER.exception("Failed to remove temporary upload file %s", tmp_path)

src/newsdom_api/mineru_runner.py

@@ -68,9 +68,11 @@ def _resolve_mineru_bin() -> str:
         return configured
     found = _cached_which("mineru")
     if not found:
-        raise FileNotFoundError(
-            "Could not find 'mineru' executable. "
-            "Ensure it is installed and on the PATH, or set NEWSDOM_MINERU_BIN."
+        raise MineruRuntimeUnavailableError(
+            stderr=(
+                "Could not find 'mineru' executable. "
+                "Ensure it is installed and on the PATH, or set NEWSDOM_MINERU_BIN."
+            )
         )
     return found
 

tests/test_mineru_runner_paths.py

@@ -61,9 +61,13 @@ def test_resolve_mineru_bin_rechecks_env_after_cached_lookup(monkeypatch):
 def test_resolve_mineru_bin_raises_when_not_found(monkeypatch):
     monkeypatch.delenv("NEWSDOM_MINERU_BIN", raising=False)
     monkeypatch.setattr(mineru_runner.shutil, "which", lambda name: None)
-    with pytest.raises(FileNotFoundError):
+    with pytest.raises(MineruRuntimeUnavailableError) as exc_info:
         mineru_runner._resolve_mineru_bin()
 
+    assert exc_info.value.returncode is None
+    assert "Could not find 'mineru' executable" in (exc_info.value.stderr or "")
+    _assert_no_private_path_material(str(exc_info.value))
+
 
 def test_find_output_dir_raises_when_missing(tmp_path: Path):
     with pytest.raises(FileNotFoundError):

tests/test_parse_endpoint.py

@@ -190,6 +190,30 @@ def fake_parse_pdf_bytes(file_path, filename):
     assert response.status_code == 200
 
 
+def test_parse_endpoint_logs_tempfile_cleanup_failure(monkeypatch, caplog):
+    def fake_parse_pdf_bytes(file_path, filename):
+        return {"document_id": "fixture", "pages": []}
+
+    def failing_unlink(self, missing_ok=False):
+        raise OSError("locked temp file")
+
+    caplog.set_level("ERROR", logger="newsdom_api")
+    monkeypatch.setattr("newsdom_api.main._validate_pdf_structure", lambda _: None)
+    monkeypatch.setattr("newsdom_api.main.parse_pdf", fake_parse_pdf_bytes)
+    monkeypatch.setattr(Path, "unlink", failing_unlink)
+
+    client = TestClient(app)
+    response = client.post(
+        "/parse",
+        files={"file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf")},
+    )
+
+    assert response.status_code == 200
+    assert response.json()["document_id"] == "fixture"
+    assert response.json()["pages"] == []
+    assert "Failed to remove temporary upload file" in caplog.text
+
+
 def test_parse_endpoint_returns_503_for_mineru_runtime_failure(monkeypatch):
     def fake_run(cmd, check, capture_output, text, timeout=None, shell=False):
         assert check is True