diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fa2b2f..b28905a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 038b7a2..a94c035 100644 --- a/README.md +++ b/README.md @@ -75,9 +75,10 @@ Output: | `noirdoc redact ` | Redact one or more files (accepts directories; `-o FILE` or `--output-dir DIR`). | | `noirdoc reveal ` | Reverse pseudonyms back to originals (DOCX / XLSX / plain; `--namespace` required). | | `noirdoc lookup ` | Resolve a pseudonym like `<>` to its original value. | -| `noirdoc ns list` | List persistent namespaces under `~/.noirdoc/namespaces/`. | -| `noirdoc ns show ` | Print the mapping summary for a namespace as JSON. | -| `noirdoc ns delete ` | Delete a namespace (prompts for confirmation). | +| `noirdoc ns list` | List persistent namespaces. | +| `noirdoc ns summary ` | Counts-only summary (entity totals + per-type counts). Safe to log. | +| `noirdoc ns show --unsafe` | Print the full pseudonym↔original mapping as JSON. **Reveals every original value.** Requires `--unsafe`. | +| `noirdoc ns delete ` | Delete a namespace (prompts for confirmation). | | `noirdoc models pull` | Download spaCy models and (optionally) GLiNER weights up front. | Run `noirdoc --help` for the full flag list on any subcommand. diff --git a/pyproject.toml b/pyproject.toml index 66318cf..c96c0c9 100644 --- a/pyproject.toml +++ b/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", diff --git a/src/noirdoc/cli.py b/src/noirdoc/cli.py index 4962e23..fcddbf2 100644 --- a/src/noirdoc/cli.py +++ b/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,6 +502,31 @@ 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, *, @@ -490,12 +534,19 @@ def _choose_output_path( 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__": diff --git a/src/noirdoc/daemon/client.py b/src/noirdoc/daemon/client.py index 56a2a1b..369a5d0 100644 --- a/src/noirdoc/daemon/client.py +++ b/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): diff --git a/src/noirdoc/daemon/protocol.py b/src/noirdoc/daemon/protocol.py index 0768717..2798946 100644 --- a/src/noirdoc/daemon/protocol.py +++ b/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): diff --git a/src/noirdoc/daemon/server.py b/src/noirdoc/daemon/server.py index b2269f9..2ed4874 100644 --- a/src/noirdoc/daemon/server.py +++ b/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) diff --git a/src/noirdoc/detection/ensemble.py b/src/noirdoc/detection/ensemble.py index c71dff8..e30994e 100644 --- a/src/noirdoc/detection/ensemble.py +++ b/src/noirdoc/detection/ensemble.py @@ -3,8 +3,12 @@ import asyncio import re +import structlog + from noirdoc.detection.base import BaseDetector, DetectedEntity +log = structlog.get_logger(__name__) + # Strong indicators: if ANY of these appear in a multi-word PERSON entity, reject it. _PERSON_STRONG_REJECT: set[str] = { # verbs / participles commonly absorbed by spaCy NER @@ -103,14 +107,11 @@ async def detect(self, text: str, language: str = "de") -> list[DetectedEntity]: return [] results = await asyncio.gather( - *(d.detect(text, language) for d in self.detectors), - return_exceptions=True, + *(self._run_one(d, text, language) for d in self.detectors), ) all_entities: list[DetectedEntity] = [] for result in results: - if isinstance(result, BaseException): - continue all_entities.extend(result) filtered = [ @@ -122,6 +123,30 @@ async def detect(self, text: str, language: str = "de") -> list[DetectedEntity]: validated = [e for e in merged if _validate_person(e)] return sorted(validated, key=lambda e: e.start) + @staticmethod + async def _run_one( + detector: BaseDetector, + text: str, + language: str, + ) -> list[DetectedEntity]: + """Run one detector. On failure, log and degrade to empty results. + + A silent ``return_exceptions=True`` would bury detector failures + and cause silent leakage (e.g. PERSON detection going dark on a + spaCy load error). We log explicitly so operators can spot the + degraded state. + """ + try: + return await detector.detect(text, language) + except Exception as exc: + log.warning( + "detection.detector_failed", + detector=getattr(detector, "name", detector.__class__.__name__), + language=language, + error=str(exc), + ) + return [] + def _merge_entities(self, entities: list[DetectedEntity]) -> list[DetectedEntity]: """ Overlap Resolution: diff --git a/src/noirdoc/file_analysis/extractors/_zip_safety.py b/src/noirdoc/file_analysis/extractors/_zip_safety.py new file mode 100644 index 0000000..441d2d4 --- /dev/null +++ b/src/noirdoc/file_analysis/extractors/_zip_safety.py @@ -0,0 +1,52 @@ +"""Zip-bomb defenses for OOXML container formats (DOCX, XLSX). + +DOCX and XLSX are zip archives. python-docx / openpyxl will happily +decompress whatever the central directory advertises, which lets a tiny +file balloon into gigabytes during extraction. We pre-flight every input +through this guard so the extractor never touches a payload we can spot +as malicious before unzip. +""" + +from __future__ import annotations + +import io +import zipfile + +# Sized for "real, large business documents are fine" — a 200 MB +# uncompressed corpus is generous; legitimate DOCX/XLSX rarely exceed +# tens of MB even with embedded media. +_MAX_UNCOMPRESSED_BYTES = 200 * 1024 * 1024 + +# Real OOXML files compress well, but ratios above ~30× are unusual. +# Cap at 100× so we block obvious bombs without false-flagging the +# occasional CSV-like sheet of repetitive text. +_MAX_COMPRESSION_RATIO = 100 + + +def check_ooxml_zip_safe(data: bytes, *, label: str) -> None: + """Raise ``ValueError`` if ``data`` looks like a zip bomb. + + Validates the central directory's declared sizes before any extractor + starts decompressing entries. + """ + try: + with zipfile.ZipFile(io.BytesIO(data)) as zf: + entries = zf.infolist() + except zipfile.BadZipFile as exc: + raise ValueError(f"{label}: not a valid zip archive") from exc + + total_uncompressed = sum(zi.file_size for zi in entries) + total_compressed = sum(zi.compress_size for zi in entries) + + if total_uncompressed > _MAX_UNCOMPRESSED_BYTES: + raise ValueError( + f"{label}: archive declares {total_uncompressed} bytes uncompressed " + f"(cap is {_MAX_UNCOMPRESSED_BYTES})", + ) + + if total_compressed and total_uncompressed / total_compressed > _MAX_COMPRESSION_RATIO: + ratio = total_uncompressed / total_compressed + raise ValueError( + f"{label}: compression ratio {ratio:.0f}x exceeds {_MAX_COMPRESSION_RATIO}x — " + "refusing as zip bomb", + ) diff --git a/src/noirdoc/file_analysis/extractors/docx_ext.py b/src/noirdoc/file_analysis/extractors/docx_ext.py index 597c51f..a79a6f7 100644 --- a/src/noirdoc/file_analysis/extractors/docx_ext.py +++ b/src/noirdoc/file_analysis/extractors/docx_ext.py @@ -4,24 +4,57 @@ import io +from noirdoc.file_analysis.extractors._zip_safety import check_ooxml_zip_safe -def extract_docx(data: bytes) -> str: - """Extract text from a DOCX byte-string (paragraphs + table cells).""" - from docx import Document - doc = Document(io.BytesIO(data)) - parts: list[str] = [] +def _walk_block_container(container, parts: list[str]) -> None: + """Append non-empty text from every paragraph and table cell in *container*. - for para in doc.paragraphs: + Used for the document body, headers, footers, and comments — all of + which are BlockItemContainers in python-docx and routinely carry the + same PII (author lines, "Confidential — Anna Müller" stamps, etc.) + the body does. + """ + for para in container.paragraphs: text = para.text.strip() if text: parts.append(text) - - for table in doc.tables: + for table in container.tables: for row in table.rows: for cell in row.cells: - text = cell.text.strip() - if text: - parts.append(text) + for cell_para in cell.paragraphs: + cell_text = cell_para.text.strip() + if cell_text: + parts.append(cell_text) + + +def extract_docx(data: bytes) -> str: + """Extract text from a DOCX byte-string. + + Walks the document body, all section headers and footers (default, + first-page, even-page), and review comments. Headers, footers, and + comments are common PII surfaces that the detector pipeline must see + before the output is reconstructed. + """ + from docx import Document + + check_ooxml_zip_safe(data, label="docx") + doc = Document(io.BytesIO(data)) + parts: list[str] = [] + + _walk_block_container(doc, parts) + + for section in doc.sections: + for header in (section.header, section.first_page_header, section.even_page_header): + _walk_block_container(header, parts) + for footer in (section.footer, section.first_page_footer, section.even_page_footer): + _walk_block_container(footer, parts) + + try: + comments = list(doc.comments) + except Exception: + comments = [] + for comment in comments: + _walk_block_container(comment, parts) return "\n".join(parts) diff --git a/src/noirdoc/file_analysis/extractors/ocr.py b/src/noirdoc/file_analysis/extractors/ocr.py index bcf9016..0e9fa19 100644 --- a/src/noirdoc/file_analysis/extractors/ocr.py +++ b/src/noirdoc/file_analysis/extractors/ocr.py @@ -3,12 +3,17 @@ from __future__ import annotations import io +import warnings from typing import TYPE_CHECKING if TYPE_CHECKING: from PIL.Image import Image as PILImage _MAX_DIM = 4096 +# Pillow emits DecompressionBombWarning above MAX_IMAGE_PIXELS and +# raises DecompressionBombError above 2x. We treat both as fatal so a +# malicious input cannot trigger huge memory allocations. +_MAX_IMAGE_PIXELS = 50_000_000 def ocr_image(img: PILImage, *, lang: str = "deu+eng") -> str: @@ -26,8 +31,23 @@ def ocr_image(img: PILImage, *, lang: str = "deu+eng") -> str: def extract_ocr(data: bytes, *, lang: str = "deu+eng") -> str: - """Run Tesseract OCR on an image byte-string.""" + """Run Tesseract OCR on an image byte-string. + + Raises ``ValueError`` if the image exceeds the decompression-bomb + threshold; the caller treats this as an extraction failure. + """ from PIL import Image - img = Image.open(io.BytesIO(data)) + previous_max = Image.MAX_IMAGE_PIXELS + Image.MAX_IMAGE_PIXELS = _MAX_IMAGE_PIXELS + try: + with warnings.catch_warnings(): + warnings.simplefilter("error", Image.DecompressionBombWarning) + img = Image.open(io.BytesIO(data)) + img.load() + except (Image.DecompressionBombError, Image.DecompressionBombWarning) as exc: + raise ValueError(f"image refused: decompression bomb suspected ({exc})") from exc + finally: + Image.MAX_IMAGE_PIXELS = previous_max + return ocr_image(img, lang=lang) diff --git a/src/noirdoc/file_analysis/extractors/pdf.py b/src/noirdoc/file_analysis/extractors/pdf.py index a84d646..247b5cb 100644 --- a/src/noirdoc/file_analysis/extractors/pdf.py +++ b/src/noirdoc/file_analysis/extractors/pdf.py @@ -7,16 +7,47 @@ logger = structlog.get_logger() +_METADATA_KEYS = ("Title", "Author", "Subject", "Keywords", "Creator", "Producer") + + +def _extract_pdf_metadata(pdf) -> str: + """Return PDF Info-dict metadata as text so detectors can see embedded PII. + + PDF /Info entries (Author, Title, …) routinely carry the same names and + addresses the body does. Without this, redacted PDFs round-trip the + metadata untouched and leak originals. + """ + lines: list[str] = [] + for key in _METADATA_KEYS: + try: + value = pdf.get_metadata_value(key) + except Exception: + continue + if value: + lines.append(f"{key}: {value}") + return "\n".join(lines) + + +def _prepend_metadata(pages_text: str, metadata_text: str) -> str: + if not metadata_text: + return pages_text + if not pages_text: + return metadata_text + return f"{metadata_text}\n\n{pages_text}" + + def extract_pdf(data: bytes, *, max_pages: int = 50) -> str: """Extract text from a PDF byte-string, page by page. - Returns concatenated text with double-newlines between pages. + Returns concatenated text with double-newlines between pages, prefixed + by any Info-dict metadata so detectors can scrub PII embedded there. """ import pypdfium2 as pdfium pdf = pdfium.PdfDocument(data) pages: list[str] = [] try: + metadata_text = _extract_pdf_metadata(pdf) for i, page in enumerate(pdf): if i >= max_pages: break @@ -26,7 +57,7 @@ def extract_pdf(data: bytes, *, max_pages: int = 50) -> str: page.close() finally: pdf.close() - return "\n\n".join(pages) + return _prepend_metadata("\n\n".join(pages), metadata_text) def extract_pdf_with_ocr_fallback( @@ -51,6 +82,7 @@ def extract_pdf_with_ocr_fallback( pages: list[str] = [] ocr_triggered = 0 try: + metadata_text = _extract_pdf_metadata(pdf) for i, page in enumerate(pdf): if i >= max_pages: break @@ -76,4 +108,4 @@ def extract_pdf_with_ocr_fallback( threshold=min_chars_per_page, ) - return "\n\n".join(pages) + return _prepend_metadata("\n\n".join(pages), metadata_text) diff --git a/src/noirdoc/file_analysis/extractors/xlsx.py b/src/noirdoc/file_analysis/extractors/xlsx.py index 8d84996..7e886ac 100644 --- a/src/noirdoc/file_analysis/extractors/xlsx.py +++ b/src/noirdoc/file_analysis/extractors/xlsx.py @@ -1,14 +1,25 @@ -"""XLSX text extraction using openpyxl.""" +"""XLSX text extraction using openpyxl. + +This helper flattens an entire workbook into a single string and is intended +for the "ship XLSX as text to a non-Excel-aware LLM" path +(``file_analysis.pipeline.convert_unsupported_files``). It is **not** suitable +for redaction — the joined text destroys cell context. Redaction must go +through :func:`noirdoc.file_analysis.xlsx_inference.pseudonymize_xlsx_smart`, +which preserves columns and writes per-cell pseudonyms. +""" from __future__ import annotations import io +from noirdoc.file_analysis.extractors._zip_safety import check_ooxml_zip_safe + def extract_xlsx(data: bytes) -> str: """Extract cell values from all sheets of an XLSX byte-string.""" from openpyxl import load_workbook + check_ooxml_zip_safe(data, label="xlsx") wb = load_workbook(io.BytesIO(data), read_only=True, data_only=True) lines: list[str] = [] try: diff --git a/src/noirdoc/file_analysis/reconstruction.py b/src/noirdoc/file_analysis/reconstruction.py index ce46e07..d9bc1bb 100644 --- a/src/noirdoc/file_analysis/reconstruction.py +++ b/src/noirdoc/file_analysis/reconstruction.py @@ -70,8 +70,25 @@ def _reconstruct_plain(block: FileBlock) -> bytes: # --------------------------------------------------------------------------- +def _replace_in_block_container(container, replacements: dict[str, str]) -> None: + """Apply *replacements* to every paragraph in *container* and its tables.""" + for para in container.paragraphs: + _replace_in_paragraph(para, replacements) + for table in container.tables: + for row in table.rows: + for cell in row.cells: + for para in cell.paragraphs: + _replace_in_paragraph(para, replacements) + + def _reconstruct_docx(block: FileBlock) -> bytes | None: - """Find-and-replace detected entities in DOCX paragraph runs.""" + """Find-and-replace detected entities in DOCX paragraph runs. + + Covers the document body, every section's headers and footers + (default, first-page, even-page variants), and review comments — + matching the surfaces walked by :func:`extract_docx` so PII the + detector saw is also stripped from the output bytes. + """ from docx import Document try: @@ -83,13 +100,20 @@ def _reconstruct_docx(block: FileBlock) -> bytes | None: if not replacements: return block.content_bytes # nothing to replace - for para in doc.paragraphs: - _replace_in_paragraph(para, replacements) - for table in doc.tables: - for row in table.rows: - for cell in row.cells: - for para in cell.paragraphs: - _replace_in_paragraph(para, replacements) + _replace_in_block_container(doc, replacements) + + for section in doc.sections: + for header in (section.header, section.first_page_header, section.even_page_header): + _replace_in_block_container(header, replacements) + for footer in (section.footer, section.first_page_footer, section.even_page_footer): + _replace_in_block_container(footer, replacements) + + try: + comments = list(doc.comments) + except Exception: + comments = [] + for comment in comments: + _replace_in_block_container(comment, replacements) buf = io.BytesIO() doc.save(buf) diff --git a/src/noirdoc/file_analysis/xlsx_inference.py b/src/noirdoc/file_analysis/xlsx_inference.py index 759ed12..40f1d2d 100644 --- a/src/noirdoc/file_analysis/xlsx_inference.py +++ b/src/noirdoc/file_analysis/xlsx_inference.py @@ -136,9 +136,12 @@ async def pseudonymize_xlsx_smart( """ from openpyxl import load_workbook + from noirdoc.file_analysis.extractors._zip_safety import check_ooxml_zip_safe + result = XlsxResult() try: + check_ooxml_zip_safe(data, label="xlsx") wb = load_workbook(io.BytesIO(data)) except Exception as exc: logger.warning("xlsx_inference.load_failed", error=str(exc)) diff --git a/src/noirdoc/namespace.py b/src/noirdoc/namespace.py index 06944c6..ede31bb 100644 --- a/src/noirdoc/namespace.py +++ b/src/noirdoc/namespace.py @@ -11,6 +11,7 @@ import json import os +import re from pathlib import Path from cryptography.fernet import Fernet, InvalidToken @@ -21,15 +22,27 @@ _KEY_FILE = "key" _DATA_FILE = "mapper.enc" +# Restrictive whitelist for namespace names. Rejects path traversal +# (``..``), absolute paths, separators, and shell metacharacters so a +# user-supplied namespace cannot escape the configured root. +_NAMESPACE_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$") + + +def _validate_namespace_name(name: str) -> str: + if not _NAMESPACE_NAME_RE.fullmatch(name): + raise ValueError( + f"invalid namespace name {name!r}: must match [A-Za-z0-9][A-Za-z0-9._-]{{0,63}}", + ) + return name class Namespace: """A persistent, Fernet-encrypted pseudonym mapping on disk.""" def __init__(self, name: str, root: Path | str | None = None) -> None: - self.name = name + self.name = _validate_namespace_name(name) self.root = Path(root).expanduser() if root else DEFAULT_NAMESPACE_ROOT - self.path = self.root / name + self.path = self.root / self.name @property def key_path(self) -> Path: @@ -46,10 +59,32 @@ def _ensure_key(self) -> bytes: if self.key_path.is_file(): return self.key_path.read_bytes() - self.path.mkdir(parents=True, exist_ok=True) + # Create the namespace directory with 0700 *up front* so the key + # file is never momentarily readable by another user. mkdir's + # ``mode`` is honored only on creation; umask may have stripped + # bits, so chmod explicitly afterward. + self.path.mkdir(parents=True, mode=0o700, exist_ok=True) + try: + os.chmod(self.path, 0o700) + except OSError: + pass + key = Fernet.generate_key() - self.key_path.write_bytes(key) - os.chmod(self.key_path, 0o600) + # Atomically create the key file with mode 0600 and refuse to + # clobber. O_EXCL closes the TOCTOU window where the file might + # have appeared after the is_file() check above. + try: + fd = os.open( + str(self.key_path), + os.O_WRONLY | os.O_CREAT | os.O_EXCL, + 0o600, + ) + except FileExistsError: + return self.key_path.read_bytes() + try: + os.write(fd, key) + finally: + os.close(fd) return key def load(self) -> PseudonymMapper: diff --git a/tests/daemon/test_path_trust.py b/tests/daemon/test_path_trust.py new file mode 100644 index 0000000..035ebf3 --- /dev/null +++ b/tests/daemon/test_path_trust.py @@ -0,0 +1,77 @@ +"""NDS-028: daemon must reject input/output paths owned by another UID.""" + +from __future__ import annotations + +import os + +import pytest + +from noirdoc.daemon import server + +pytestmark = pytest.mark.asyncio + + +async def test_redact_rejects_input_owned_by_another_uid(monkeypatch, tmp_path): + """A peer that asks the daemon to read a file not owned by its UID is refused.""" + target = tmp_path / "victim.txt" + target.write_bytes(b"some content") + + # Pretend our UID is something other than the file's owner. + monkeypatch.setattr(os, "getuid", lambda: os.stat(target).st_uid + 1) + + state = server.DaemonState() + params = { + "input": {"type": "file", "path": str(target)}, + "output_path": str(tmp_path / "out.txt"), + } + with pytest.raises(ValueError, match="not owned by the current user"): + await server.handle_redact(state, params) + + +async def test_redact_rejects_missing_input(tmp_path): + state = server.DaemonState() + params = { + "input": {"type": "file", "path": str(tmp_path / "nonexistent")}, + } + with pytest.raises(ValueError, match="not found"): + await server.handle_redact(state, params) + + +async def test_redact_rejects_output_parent_owned_by_another_uid(monkeypatch, tmp_path): + """Refuse to write into a directory owned by a different UID.""" + src = tmp_path / "in.txt" + src.write_bytes(b"content") + bad_dir = tmp_path / "owned-by-someone-else" + bad_dir.mkdir() + + real_stat = os.stat + real_uid = os.getuid() + + def fake_stat(path, *args, **kwargs): + st = real_stat(path, *args, **kwargs) + if str(path).endswith("owned-by-someone-else"): + return os.stat_result( + ( + st.st_mode, + st.st_ino, + st.st_dev, + st.st_nlink, + real_uid + 1, # foreign uid + st.st_gid, + st.st_size, + st.st_atime, + st.st_mtime, + st.st_ctime, + ), + ) + return st + + monkeypatch.setattr(server.os, "stat", fake_stat) + + state = server.DaemonState() + params = { + "input": {"type": "file", "path": str(src)}, + "output_path": str(bad_dir / "result.txt"), + } + with pytest.raises(ValueError, match="not owned by the current user"): + await server.handle_redact(state, params) diff --git a/tests/daemon/test_protocol.py b/tests/daemon/test_protocol.py index b54fdf7..97ef263 100644 --- a/tests/daemon/test_protocol.py +++ b/tests/daemon/test_protocol.py @@ -7,6 +7,8 @@ import pytest from noirdoc.daemon.protocol import ( + MAX_PATH_LEN, + MAX_TEXT_VALUE_LEN, ErrorPayload, HelloParams, HelloResult, @@ -108,3 +110,17 @@ def test_response_envelope_with_error(): out = _roundtrip(resp) assert out.error is not None assert out.error.code == "bad_request" + + +def test_oversized_text_value_rejected(): + """Text inputs above MAX_TEXT_VALUE_LEN are refused to bound DoS.""" + too_big = "a" * (MAX_TEXT_VALUE_LEN + 1) + with pytest.raises(Exception): + RedactTextInput(value=too_big) + + +def test_oversized_path_rejected(): + """Paths above MAX_PATH_LEN are refused to bound DoS.""" + too_long = "/" + ("a" * MAX_PATH_LEN) + with pytest.raises(Exception): + RedactFileInput(path=too_long) diff --git a/tests/file_analysis/test_docx_extractor.py b/tests/file_analysis/test_docx_extractor.py new file mode 100644 index 0000000..4a4ff58 --- /dev/null +++ b/tests/file_analysis/test_docx_extractor.py @@ -0,0 +1,99 @@ +"""NDS-016: DOCX extraction must cover header/footer/comment surfaces.""" + +from __future__ import annotations + +import io + +from noirdoc.detection.base import DetectedEntity +from noirdoc.file_analysis.extractors.docx_ext import extract_docx +from noirdoc.file_analysis.models import FileBlock +from noirdoc.file_analysis.reconstruction import _reconstruct_docx + + +def _entity(text: str, in_text: str) -> DetectedEntity: + start = in_text.index(text) + return DetectedEntity( + entity_type="PERSON", + text=text, + start=start, + end=start + len(text), + score=0.9, + source="test", + ) + + +def _docx_with_headers_footers_and_body() -> bytes: + """Build a DOCX whose header, footer, and body each contain distinct PII.""" + from docx import Document + + doc = Document() + section = doc.sections[0] + section.header.paragraphs[0].text = "Header: Anna Mueller" + section.footer.paragraphs[0].text = "Footer: Bernd Schmidt" + doc.add_paragraph("Body: Carla Weber") + + buf = io.BytesIO() + doc.save(buf) + return buf.getvalue() + + +def test_extract_docx_walks_headers_and_footers(): + """PII embedded in section headers and footers must reach the detector.""" + text = extract_docx(_docx_with_headers_footers_and_body()) + assert "Anna Mueller" in text + assert "Bernd Schmidt" in text + assert "Carla Weber" in text + + +def test_extract_docx_walks_comments(): + """Review comments are a routine PII surface — they must be extracted.""" + from docx import Document + + doc = Document() + para = doc.add_paragraph("Body text") + doc.add_comment(runs=[para.runs[0]] if para.runs else [], text="Reviewer: Dora Klein") + buf = io.BytesIO() + doc.save(buf) + + text = extract_docx(buf.getvalue()) + assert "Dora Klein" in text + + +def test_reconstruct_docx_replaces_text_in_headers_and_footers(): + """Reconstruction must scrub header/footer text so the output bytes are clean.""" + docx_bytes = _docx_with_headers_footers_and_body() + extracted = extract_docx(docx_bytes) + + # Build a synthetic pseudonymized result: replace each name with a token + pseudonymized = ( + extracted.replace("Anna Mueller", "<>") + .replace("Bernd Schmidt", "<>") + .replace("Carla Weber", "<>") + ) + + entities = [ + _entity("Anna Mueller", extracted), + _entity("Bernd Schmidt", extracted), + _entity("Carla Weber", extracted), + ] + + block = FileBlock( + content_bytes=docx_bytes, + mime_type="application/vnd.openxmlformats-officedocument.wordprocessingml.document", + source_path="test.docx", + source_type="file", + extracted_text=extracted, + pseudonymized_text=pseudonymized, + entities=entities, + ) + + new_bytes = _reconstruct_docx(block) + assert new_bytes is not None + + rewritten = extract_docx(new_bytes) + assert "Anna Mueller" not in rewritten + assert "Bernd Schmidt" not in rewritten + assert "Carla Weber" not in rewritten + assert "<>" in rewritten + assert "<>" in rewritten + assert "<>" in rewritten diff --git a/tests/file_analysis/test_extractor.py b/tests/file_analysis/test_extractor.py index 3f3ad8b..22d634a 100644 --- a/tests/file_analysis/test_extractor.py +++ b/tests/file_analysis/test_extractor.py @@ -71,3 +71,29 @@ async def test_corrupt_data_sets_error(extractor): result = await extractor.extract_text(block) assert result is None assert block.extraction_error is not None + + +async def test_ocr_decompression_bomb_refused(monkeypatch): + """A PNG that decodes to >50 MP must be refused, not OOM the process.""" + import io + + from PIL import Image + + # Force the bomb threshold low so the test stays fast; the production + # constant is 50 MP. + monkeypatch.setattr( + "noirdoc.file_analysis.extractors.ocr._MAX_IMAGE_PIXELS", + 100, + ) + huge = Image.new("RGB", (50, 50), color="white") + buf = io.BytesIO() + huge.save(buf, format="PNG") + block = FileBlock( + content_bytes=buf.getvalue(), + mime_type="image/png", + source_path="test", + source_type="image", + ) + result = await FileTextExtractor(ocr_enabled=True).extract_text(block) + assert result is None + assert "decompression bomb" in (block.extraction_error or "").lower() diff --git a/tests/file_analysis/test_pdf_extractor.py b/tests/file_analysis/test_pdf_extractor.py index c36f097..e0d3aee 100644 --- a/tests/file_analysis/test_pdf_extractor.py +++ b/tests/file_analysis/test_pdf_extractor.py @@ -83,6 +83,40 @@ def _make_image_pdf(text: str) -> bytes: return buf.getvalue() +def _make_pdf_with_metadata( + body_text: str, + *, + author: str | None = None, + title: str | None = None, + subject: str | None = None, +) -> bytes: + """A single-page PDF carrying /Info metadata fields. + + Used to verify that PDF /Info entries are surfaced to the detector so PII + embedded there gets pseudonymized rather than passed through. + """ + from PIL import Image, ImageDraw, ImageFont + + img = Image.new("RGB", (1200, 1600), "white") + draw = ImageDraw.Draw(img) + try: + font = ImageFont.truetype("/System/Library/Fonts/Helvetica.ttc", 48) + except OSError: + font = ImageFont.load_default() + draw.text((60, 100), body_text, fill="black", font=font) + + buf = io.BytesIO() + save_kwargs: dict[str, str] = {} + if author is not None: + save_kwargs["author"] = author + if title is not None: + save_kwargs["title"] = title + if subject is not None: + save_kwargs["subject"] = subject + img.save(buf, format="PDF", resolution=150.0, **save_kwargs) + return buf.getvalue() + + @pytest.fixture def text_pdf_bytes() -> bytes: return _TEXT_PDF_BYTES @@ -107,6 +141,27 @@ def test_extract_pdf_returns_empty_for_image_only_pdf(image_pdf_bytes): assert text.strip() == "" +def test_extract_pdf_surfaces_info_dict_metadata(): + """PII in /Info fields must reach the extracted text so detection sees it.""" + pdf_bytes = _make_pdf_with_metadata( + "body text", + author="Anna Mueller", + title="Vertrag fuer Mueller", + subject="Confidential", + ) + text = extract_pdf(pdf_bytes) + assert "Author: Anna Mueller" in text + assert "Title: Vertrag fuer Mueller" in text + assert "Subject: Confidential" in text + + +def test_extract_pdf_handles_missing_metadata(text_pdf_bytes): + """A PDF without /Info entries must not crash or inject spurious labels.""" + text = extract_pdf(text_pdf_bytes) + assert "Author:" not in text + assert "Title:" not in text + + # ── extract_pdf_with_ocr_fallback (new) ────────────────────────────────────── diff --git a/tests/file_analysis/test_zip_safety.py b/tests/file_analysis/test_zip_safety.py new file mode 100644 index 0000000..f7a2d56 --- /dev/null +++ b/tests/file_analysis/test_zip_safety.py @@ -0,0 +1,46 @@ +"""NDS-015: refuse OOXML inputs whose zip envelope looks like a bomb.""" + +from __future__ import annotations + +import io +import zipfile + +import pytest + +from noirdoc.file_analysis.extractors._zip_safety import check_ooxml_zip_safe + + +def _build_zip(entries: list[tuple[str, bytes]], *, compress: bool = True) -> bytes: + """Build a zip archive with the given (name, payload) entries.""" + buf = io.BytesIO() + method = zipfile.ZIP_DEFLATED if compress else zipfile.ZIP_STORED + with zipfile.ZipFile(buf, mode="w", compression=method) as zf: + for name, payload in entries: + zf.writestr(name, payload) + return buf.getvalue() + + +def test_check_ooxml_passes_for_normal_archive(): + payload = _build_zip([("word/document.xml", b"hello")]) + check_ooxml_zip_safe(payload, label="docx") + + +def test_check_ooxml_rejects_non_zip_input(): + with pytest.raises(ValueError, match="not a valid zip archive"): + check_ooxml_zip_safe(b"not a zip", label="docx") + + +def test_check_ooxml_rejects_oversized_uncompressed(): + """A single highly-compressible 300 MB blob must be refused.""" + bomb = b"A" * (300 * 1024 * 1024) + payload = _build_zip([("payload.bin", bomb)]) + with pytest.raises(ValueError, match="bytes uncompressed"): + check_ooxml_zip_safe(payload, label="xlsx") + + +def test_check_ooxml_rejects_extreme_compression_ratio(): + """A 10 MB blob of zeros compresses to a tiny file — ratio guard catches it.""" + bomb = b"\x00" * (10 * 1024 * 1024) + payload = _build_zip([("payload.bin", bomb)]) + with pytest.raises(ValueError, match="compression ratio"): + check_ooxml_zip_safe(payload, label="docx") diff --git a/tests/test_cli.py b/tests/test_cli.py index df0beb5..b64b6f5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,6 +3,8 @@ import json from pathlib import Path +import click +import pytest from click.testing import CliRunner from noirdoc import cli as cli_module @@ -46,3 +48,63 @@ def test_ns_summary_missing_namespace(monkeypatch, tmp_path: Path): result = CliRunner().invoke(main, ["ns", "summary", "nope"]) assert result.exit_code == 1 assert "Namespace 'nope' does not exist." in result.output + + +def test_ns_show_requires_unsafe_flag(monkeypatch, tmp_path: Path): + """ns show must not print original values without --unsafe.""" + _redirect_namespace_root(monkeypatch, tmp_path) + + ns = Namespace("demo") + mapper = ns.load() + mapper.get_or_create("Anna Müller", "PERSON") + ns.save(mapper) + + result = CliRunner().invoke(main, ["ns", "show", "demo"]) + assert result.exit_code == 2 + assert "Anna Müller" not in result.output + assert "--unsafe" in result.output + + +def test_ns_show_unsafe_prints_mapping(monkeypatch, tmp_path: Path): + _redirect_namespace_root(monkeypatch, tmp_path) + + ns = Namespace("demo") + mapper = ns.load() + mapper.get_or_create("Anna Müller", "PERSON") + ns.save(mapper) + + result = CliRunner().invoke(main, ["ns", "show", "demo", "--unsafe"]) + assert result.exit_code == 0, result.output + assert "Anna Müller" in result.output + + +def test_choose_output_path_drops_input_directory_components(tmp_path: Path): + """Crafted input paths must not route output outside --output-dir.""" + out_dir = tmp_path / "out" + out_dir.mkdir() + crafted = tmp_path / "in" / ".." / "etc" / "passwd-fake" + crafted.parent.mkdir(parents=True, exist_ok=True) + crafted.touch() + chosen = cli_module._choose_output_path( + crafted, + output=None, + output_dir=out_dir, + reconstructed=True, + ) + assert chosen.is_relative_to(out_dir.resolve()) + + +def test_choose_output_path_refuses_namespace_store(monkeypatch, tmp_path: Path): + """Refuse to overwrite anything inside the namespaces directory.""" + namespaces_root = tmp_path / "namespaces" + namespaces_root.mkdir() + monkeypatch.setattr(cli_module, "DEFAULT_NAMESPACE_ROOT", namespaces_root) + + target = namespaces_root / "demo" / "key" + with pytest.raises(click.ClickException, match="namespace store"): + cli_module._choose_output_path( + tmp_path / "vertrag.txt", + output=target, + output_dir=None, + reconstructed=True, + ) diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index a17e0a7..e352ef5 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -166,6 +166,18 @@ async def test_failing_detector_does_not_crash(): assert len(result) == 1 +async def test_failing_detector_logs_warning(capsys): + """A failing detector must log a warning so operators can detect degraded state.""" + ent = _ent("PERSON", "Max", 0, 3, 0.9, "presidio") + ensemble = EnsembleDetector( + [FailingDetector(), FakeDetector([ent])], + score_threshold=0.0, + ) + await ensemble.detect("dummy", "de") + captured = capsys.readouterr().out + capsys.readouterr().err + assert "detection.detector_failed" in captured + + # --- PERSON validation --- diff --git a/tests/test_namespace.py b/tests/test_namespace.py index 22ef348..85d69af 100644 --- a/tests/test_namespace.py +++ b/tests/test_namespace.py @@ -19,9 +19,24 @@ def test_create_namespace_generates_key(tmp_path: Path): assert mapper.entity_count == 0 assert ns.exists() assert ns.key_path.is_file() - # Key file should be 0600 + # Key file must be 0600 from the moment it is created. mode = ns.key_path.stat().st_mode & 0o777 assert mode == 0o600 + # Namespace directory must be 0700 so other users cannot list it. + dir_mode = ns.path.stat().st_mode & 0o777 + assert dir_mode == 0o700 + + +def test_existing_key_not_clobbered_on_concurrent_load(tmp_path: Path): + """A second `_ensure_key` must reuse the existing key, not overwrite it.""" + ns = Namespace("demo", root=tmp_path) + key1 = ns._ensure_key() + key2 = ns._ensure_key() + assert key1 == key2 + # Even after deleting the in-memory file handle and re-creating, the + # on-disk key remains stable. + ns2 = Namespace("demo", root=tmp_path) + assert ns2._ensure_key() == key1 def test_save_and_load_roundtrip(tmp_path: Path): @@ -84,6 +99,28 @@ def test_mapper_to_dict_roundtrip(): assert restored.get_or_create("Jane Doe", "PERSON") == "<>" +@pytest.mark.parametrize( + "bad_name", + [ + "../escape", + "/etc/passwd", + "..", + ".", + "foo/bar", + "foo\\bar", + "foo bar", + "", + "a" * 65, + ".hidden", + "-leading-dash", + ], +) +def test_namespace_rejects_unsafe_names(tmp_path: Path, bad_name: str): + """Path-traversal / shell-metacharacter names must raise before any I/O.""" + with pytest.raises(ValueError, match="invalid namespace name"): + Namespace(bad_name, root=tmp_path) + + def test_corrupt_key_raises(tmp_path: Path): ns = Namespace("demo", root=tmp_path) mapper = ns.load()