Security patch: address all High findings for v0.1.2

CHANGELOG.md

@@ -5,6 +5,60 @@ and [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+## [0.1.2] — 2026-04-27
+
+Security patch covering all High-severity findings from the 0.1.1
+internal security review. Recommended upgrade for anyone running
+0.1.x.
+
+### Security
+- **PDF metadata leak.** PII embedded in a PDF's `/Info` dictionary
+  (Author, Title, Subject, Creator, Producer, Keywords) was passed
+  through unchanged. Metadata fields are now extracted with the page
+  text so the detector ensemble pseudonymizes them before output.
+- **DOCX header / footer / comment leak.** `extract_docx` and
+  `_reconstruct_docx` only walked paragraphs and table cells — section
+  headers, footers (default + first-page + even-page), and review
+  comments survived untouched. All three surfaces are now extracted
+  on input and rewritten on output.
+- **OOXML zip-bomb defense.** DOCX and XLSX inputs are now pre-flighted
+  through a zip-envelope check that refuses archives declaring more
+  than 200 MB uncompressed or a compression ratio above 100×.
+- **XML entity expansion.** `defusedxml` is now a baseline dependency
+  so openpyxl's `iterparse` path is entity-safe by default. python-docx
+  was already pinned to a `resolve_entities=False` parser.
+- **Image decompression-bomb DoS.** OCR extraction now caps
+  `Image.MAX_IMAGE_PIXELS` at 50 megapixels and converts
+  `DecompressionBombWarning` into a hard refusal. The cap is
+  scoped per-call so it cannot leak into other PIL consumers.
+- **Detector ensemble silent failure.** A failing detector inside
+  `EnsembleDetector` no longer produces an empty result silently;
+  the failure is logged with the detector name so observability
+  catches partial-coverage regressions.
+- **Daemon protocol size limits.** `asyncio.start_unix_server` and
+  the matching client now cap line buffers at 32 MB, and the
+  protocol enforces per-field length caps (16 MB text, 4 KB paths,
+  64 chars for namespace names).
+- **Daemon path-trust check.** `handle_redact` now refuses input
+  files and output directories that are not owned by the current
+  UID. A peer cannot ask the daemon to read or overwrite another
+  user's files.
+- **Namespace name validation.** Namespace names are restricted to
+  `^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$`, blocking path traversal
+  (`../`, `/`, leading dot) into or out of the namespace store.
+- **Fernet key TOCTOU.** Per-namespace key creation now uses
+  `O_CREAT | O_EXCL` with `0600`, the namespace directory is
+  created with `0700`, and concurrent first-load races no longer
+  clobber an existing key.
+- **CLI output-path traversal.** `noirdoc redact -o / --output-dir`
+  is now guarded against crafted input paths that resolved outside
+  the chosen output directory, and refuses to write into the
+  namespace store.
+- **`ns show` requires `--unsafe`.** Printing a namespace's full
+  pseudonym ↔ original mapping reveals every original value. The
+  command now exits with an error and points at `noirdoc ns
+  summary` unless `--unsafe` is passed.
+
 ## [0.1.1] — 2026-04-27
 
 ### Added
@@ -58,6 +112,7 @@ First public alpha on PyPI.
   a `UserWarning`, and keeps working. Explicit `--detector gliner` still fails
   loudly when the `[full]` extra isn't installed.
 
-[Unreleased]: https://github.com/nextaim-de/noirdoc/compare/v0.1.1...HEAD
+[Unreleased]: https://github.com/nextaim-de/noirdoc/compare/v0.1.2...HEAD
+[0.1.2]: https://github.com/nextaim-de/noirdoc/releases/tag/v0.1.2
 [0.1.1]: https://github.com/nextaim-de/noirdoc/releases/tag/v0.1.1
 [0.1.0]: https://github.com/nextaim-de/noirdoc/releases/tag/v0.1.0

README.md

@@ -75,9 +75,10 @@ Output:
 | `noirdoc redact <files>`     | Redact one or more files (accepts directories; `-o FILE` or `--output-dir DIR`).       |
 | `noirdoc reveal <file>`      | Reverse pseudonyms back to originals (DOCX / XLSX / plain; `--namespace` required).    |
 | `noirdoc lookup <token>`     | Resolve a pseudonym like `<<PERSON_1>>` to its original value.                         |
-| `noirdoc ns list`            | List persistent namespaces under `~/.noirdoc/namespaces/`.                             |
-| `noirdoc ns show <name>`     | Print the mapping summary for a namespace as JSON.                                     |
-| `noirdoc ns delete <name>`   | Delete a namespace (prompts for confirmation).                                         |
+| `noirdoc ns list`              | List persistent namespaces.                                                            |
+| `noirdoc ns summary <name>`    | Counts-only summary (entity totals + per-type counts). Safe to log.                    |
+| `noirdoc ns show <name> --unsafe` | Print the full pseudonym↔original mapping as JSON. **Reveals every original value.** Requires `--unsafe`. |
+| `noirdoc ns delete <name>`     | Delete a namespace (prompts for confirmation).                                         |
 | `noirdoc models pull`        | Download spaCy models and (optionally) GLiNER weights up front.                        |
 
 Run `noirdoc <cmd> --help` for the full flag list on any subcommand.

pyproject.toml

@@ -50,6 +50,7 @@ dependencies = [
     "pypdfium2>=4.0.0",
     "python-docx>=1.1.0",
     "openpyxl>=3.1.0",
+    "defusedxml>=0.7.1",
     "pytesseract>=0.3.10",
     "Pillow>=10.0.0",
     "python-magic>=0.4.27",

src/noirdoc/cli.py

@@ -314,8 +314,27 @@ def ns_list() -> None:
 
 @ns.command("show")
 @click.argument("namespace")
-def ns_show(namespace: str) -> None:
-    """Print the mapping summary for NAMESPACE as JSON."""
+@click.option(
+    "--unsafe",
+    is_flag=True,
+    help="Acknowledge that the full pseudonym→original mapping will be printed.",
+)
+def ns_show(namespace: str, unsafe: bool) -> None:
+    """Print the full pseudonym→original mapping as JSON.
+
+    Reveals every original value in the namespace. Refuses to run
+    without --unsafe so the mapping cannot be captured by accident
+    (terminal scrollback, CI logs, copy-paste). For a non-revealing
+    overview, use ``noirdoc ns summary``.
+    """
+    if not unsafe:
+        click.echo(
+            "ns show prints original values that defeat redaction. "
+            "Re-run with --unsafe to confirm, or use 'noirdoc ns summary' "
+            "for a counts-only view.",
+            err=True,
+        )
+        sys.exit(2)
     ns_obj = Namespace(namespace)
     if not ns_obj.exists():
         click.echo(f"Namespace {namespace!r} does not exist.", err=True)
@@ -483,19 +502,51 @@ def _expand_inputs(inputs: tuple[Path, ...]) -> list[Path]:
     return files
 
 
+def _guard_output_path(out_path: Path, *, output_dir: Path | None) -> Path:
+    """Canonicalize ``out_path`` and refuse paths that escape sane bounds.
+
+    Two checks:
+    * If ``--output-dir`` was given, the resolved output must live under
+      the resolved output dir. Drops the bullet "malicious input
+      filename routes the redacted output outside the requested dir".
+    * Refuse anything inside the user's namespaces directory: clobbering
+      a key file would silently destroy reversibility.
+    """
+    resolved = out_path.resolve()
+    if output_dir is not None:
+        resolved_dir = output_dir.resolve()
+        if not resolved.is_relative_to(resolved_dir):
+            raise click.ClickException(
+                f"refusing to write {resolved} outside --output-dir {resolved_dir}",
+            )
+    namespaces_root = DEFAULT_NAMESPACE_ROOT.resolve()
+    if resolved.is_relative_to(namespaces_root):
+        raise click.ClickException(
+            f"refusing to write inside namespace store {namespaces_root}",
+        )
+    return resolved
+
+
 def _choose_output_path(
     input_path: Path,
     *,
     output: Path | None,
     output_dir: Path | None,
     reconstructed: bool,
 ) -> Path:
+    # Always derive the leaf name from the input *basename* so a
+    # crafted input path like "/tmp/in/../etc/passwd" cannot route the
+    # output anywhere but next to the requested location.
+    safe_name = Path(input_path.name)
     if output:
-        return output
-    parent = output_dir or input_path.parent
-    if reconstructed:
-        return parent / f"{input_path.stem}_redacted{input_path.suffix}"
-    return parent / f"{input_path.stem}_redacted.txt"
+        chosen = output
+    else:
+        parent = output_dir or input_path.parent
+        if reconstructed:
+            chosen = parent / f"{safe_name.stem}_redacted{safe_name.suffix}"
+        else:
+            chosen = parent / f"{safe_name.stem}_redacted.txt"
+    return _guard_output_path(chosen, output_dir=output_dir)
 
 
 if __name__ == "__main__":

src/noirdoc/daemon/client.py

@@ -20,6 +20,9 @@
 CONNECT_TIMEOUT = 2.0
 RPC_TIMEOUT = 600.0  # generous; covers cold-spawn warmup + slow file redaction
 SHUTDOWN_DRAIN_TIMEOUT = 5.0
+# Match the server-side cap (server.SOCKET_READ_LIMIT). Bounds the
+# memory the client will buffer if the daemon sends an oversize line.
+SOCKET_READ_LIMIT = 32 * 1024 * 1024
 
 
 class DaemonError(Exception):
@@ -35,7 +38,7 @@ async def _try_connect(
 ) -> tuple[asyncio.StreamReader, asyncio.StreamWriter] | None:
     try:
         return await asyncio.wait_for(
-            asyncio.open_unix_connection(path=str(socket_path)),
+            asyncio.open_unix_connection(path=str(socket_path), limit=SOCKET_READ_LIMIT),
             timeout=CONNECT_TIMEOUT,
         )
     except (FileNotFoundError, ConnectionRefusedError, TimeoutError, OSError):

src/noirdoc/daemon/protocol.py

@@ -20,9 +20,17 @@
 
 DetectorChoice = Literal["presidio", "gliner", "ensemble"]
 
+# Per-field caps. Bound so a malicious or buggy client cannot wedge the
+# daemon with a multi-gigabyte JSON payload. Tunable here; matched by
+# the asyncio buffer limit in server.py / client.py.
+MAX_TEXT_VALUE_LEN = 16 * 1024 * 1024  # 16 MB, covers very large texts
+MAX_PATH_LEN = 4096  # POSIX PATH_MAX
+MAX_NAMESPACE_LEN = 64
+MAX_DETECTOR_MODEL_LEN = 512
+
 
 class HelloParams(BaseModel):
-    client_version: str
+    client_version: str = Field(max_length=64)
 
 
 class HelloResult(BaseModel):
@@ -33,12 +41,12 @@ class HelloResult(BaseModel):
 
 class RedactTextInput(BaseModel):
     type: Literal["text"] = "text"
-    value: str
+    value: str = Field(max_length=MAX_TEXT_VALUE_LEN)
 
 
 class RedactFileInput(BaseModel):
     type: Literal["file"] = "file"
-    path: str  # absolute path on the daemon's filesystem (same user as CLI)
+    path: str = Field(max_length=MAX_PATH_LEN)
 
 
 RedactInput = Annotated[
@@ -48,14 +56,17 @@ class RedactFileInput(BaseModel):
 
 
 class RedactParams(BaseModel):
-    namespace: str | None = None
-    namespace_root: str | None = None
-    language: str = "de"
+    namespace: str | None = Field(default=None, max_length=MAX_NAMESPACE_LEN)
+    namespace_root: str | None = Field(default=None, max_length=MAX_PATH_LEN)
+    language: str = Field(default="de", max_length=8)
     detector: DetectorChoice = "ensemble"
     score_threshold: float = 0.5
-    gliner_model: str = "knowledgator/gliner-pii-edge-v1.0"
+    gliner_model: str = Field(
+        default="knowledgator/gliner-pii-edge-v1.0",
+        max_length=MAX_DETECTOR_MODEL_LEN,
+    )
     input: RedactInput
-    output_path: str | None = None  # for file input; daemon writes here directly
+    output_path: str | None = Field(default=None, max_length=MAX_PATH_LEN)
 
 
 class RedactResult(BaseModel):

src/noirdoc/daemon/server.py

@@ -49,6 +49,12 @@
 LOG_BACKUP_COUNT = 1
 SUPPORTED_WARMUP_LANGUAGES = ("de", "en")
 
+# Cap per-message buffer the asyncio StreamReader will accept. A line
+# longer than this raises LimitOverrunError instead of growing memory
+# unbounded. Matches the protocol-level Field(max_length=...) caps with
+# enough headroom for JSON encoding overhead.
+SOCKET_READ_LIMIT = 32 * 1024 * 1024
+
 log = logging.getLogger("noirdoc.daemon")
 
 
@@ -220,6 +226,24 @@ async def handle_shutdown(
     return ShutdownResult().model_dump()
 
 
+def _check_same_uid(path: Path, label: str) -> None:
+    """Refuse a request if *path* is not owned by the daemon's UID.
+
+    Closes the same-UID confused-deputy hole where any process able to
+    talk to the daemon socket could trick it into reading or writing
+    files the user did not intend. The daemon already runs as the user;
+    this assertion guards against symlink-swap and sloppy callers.
+    """
+    try:
+        st = os.stat(path)
+    except FileNotFoundError as exc:
+        raise ValueError(f"{label} not found: {path}") from exc
+    if st.st_uid != os.getuid():
+        raise ValueError(
+            f"{label} {path} is not owned by the current user (uid={os.getuid()})",
+        )
+
+
 async def handle_redact(
     state: DaemonState,
     params: dict[str, Any],
@@ -229,6 +253,17 @@ async def handle_redact(
 
     parsed = RedactParams.model_validate(params)
 
+    if isinstance(parsed.input, RedactFileInput):
+        in_path = Path(parsed.input.path).resolve()
+        _check_same_uid(in_path, "input.path")
+        parsed.input.path = str(in_path)
+    if parsed.output_path:
+        out_path = Path(parsed.output_path).resolve()
+        out_parent = out_path.parent
+        out_parent.mkdir(parents=True, exist_ok=True)
+        _check_same_uid(out_parent, "output_path parent")
+        parsed.output_path = str(out_path)
+
     state.queue_depth += 1
     try:
         async with state.redact_lock:
@@ -346,7 +381,7 @@ async def _dispatch(state: DaemonState, raw_line: bytes) -> Response:
 
     try:
         result = await handler(state, request.params)
-    except ValidationError as exc:
+    except (ValidationError, ValueError) as exc:
         return Response(
             id=request.id,
             error=ErrorPayload(code=ERR_BAD_REQUEST, message=str(exc)),
@@ -368,7 +403,26 @@ async def _handle_connection(
 ) -> None:
     try:
         while not state.shutdown_event.is_set():
-            line = await reader.readline()
+            try:
+                line = await reader.readline()
+            except asyncio.LimitOverrunError as exc:
+                # Drain the offending line and report the error so a
+                # malicious peer can't wedge the daemon by sending an
+                # endless line.
+                log.warning("daemon.line_too_long bytes=%d", exc.consumed)
+                writer.write(
+                    _serialize(
+                        Response(
+                            id="",
+                            error=ErrorPayload(
+                                code=ERR_BAD_REQUEST,
+                                message=f"request line exceeded {SOCKET_READ_LIMIT} bytes",
+                            ),
+                        ),
+                    ),
+                )
+                await writer.drain()
+                return
             if not line:
                 return  # client closed
             response = await _dispatch(state, line)
@@ -432,6 +486,7 @@ async def _async_main() -> None:
     server = await asyncio.start_unix_server(
         lambda r, w: _handle_connection(r, w, state),
         path=str(sock_path),
+        limit=SOCKET_READ_LIMIT,
     )
     try:
         os.chmod(sock_path, 0o600)