Skip to content

fix(inspection): stop interpolating third-party messages into adapter errors - #40

Open
mazzasaverio wants to merge 1 commit into
ma2za:mainfrom
mazzasaverio:fix/adapter-error-sanitization
Open

fix(inspection): stop interpolating third-party messages into adapter errors#40
mazzasaverio wants to merge 1 commit into
ma2za:mainfrom
mazzasaverio:fix/adapter-error-sanitization

Conversation

@mazzasaverio

Copy link
Copy Markdown
Contributor

Summary

The two new inspection adapters raise AdapterExecutionError(f"...: {e}") from e, which puts the underlying library's message into the error text and keeps the cause chain. Everywhere else the codebase deliberately does the opposite: fixed strings plus from None (see _extract_document, _render_document, and the JSONL line error in formats.py), and 0.1.0a3 even shipped a changelog entry named "Sanitized unsupported-format, decoding, parsing, rendering, and CLI failures".

What that costs today, on a file that is not a valid DOCX:

AdapterExecutionError: Failed to read Office document: Package not found at
'/home/user/clients/acme-holdings/payroll.docx'

The path alone can be sensitive in the contexts this package targets (a client name in a directory, a case number in a filename), and pdfminer messages can carry fragments of document content. Through Pseudonymizer.inspect_file the engine re-wraps these, so the leak is limited to direct adapter use, but the adapters are public and re-exported from pseudonymize.inspection.

After this change:

AdapterExecutionError: input adapter failed while reading the DOCX document

A regression test builds a broken file under a directory named after a fictional client, with an email inside, and asserts that neither the path, nor the filename, nor the content appears in the raised error, and that the cause is suppressed the way the engine does it.

Two notes

  • from None suppresses the context from tracebacks but does not clear __context__ on the object, so the test asserts __cause__ is None and __suppress_context__ rather than pretending the attribute disappears. This matches the existing convention exactly rather than inventing a stricter one.
  • Sanitized messages are worse for production debugging, which is the trade-off the project already accepted. Architecture and scalability review: current limits and proposed solutions #36 (item 10) proposes an opt-in diagnostics hook as the principled way to get that information back without putting it in the exception text; this PR does not pre-empt that discussion.

The two existing error-message tests were updated to the new wording.

Verification

  • Tests use synthetic data only
  • Ruff, mypy, and pytest pass (211 passed, coverage 99.62%, mypy clean on 70 files)
  • Documentation and changelog are updated when required
  • Security and privacy impact has been considered (this is the security fix; it strictly removes information from error text)
  • No compatibility shim was added for an alpha-only API without an explicit roadmap reason
  • The base package remains dependency-free and network-denied by default

Separate observation, not changed here because it touches the public exception surface: both adapters raise a bare RuntimeError when the extra is missing, while LocalONNXPIIBackend raises ImportError for the same situation. Worth aligning, but that is your call on which of the two wins.

🤖 Generated with Claude Code

… errors

The PDF and Office adapters raised AdapterExecutionError with the
underlying library's message embedded and the cause chained, so a failure
on an invalid document surfaced the full file path (and, for pdfminer,
possibly document fragments). In the contexts this package targets a path
is itself sensitive: /home/user/clients/acme-holdings/payroll.docx names
the client.

Both adapters now raise fixed, format-naming messages with 'from None',
matching the convention the engine and the built-in adapters already use.
A regression test asserts that neither the directory name, the filename,
nor the file content reaches the raised error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mazzasaverio
mazzasaverio requested a review from ma2za as a code owner August 22, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant