fix(api): batch errors no longer echo library internals — CWE-209 - #147
Merged
Merged
Conversation
MrChengLen
force-pushed
the
pr-batch-error-messages
branch
from
September 26, 2026 13:33
220392b to
9a7b39c
Compare
/convert/batch and /compress/batch returned str(e) for any ValueError.
Library exceptions are ValueErrors too (UnicodeDecodeError from the strict
UTF-8 reads in the Markdown/CSV/JSON converters, JSONDecodeError, some
Pillow errors), so their internals (codec, byte offset, parser position)
reached the per-file error message, the X-FileMorph-Batch-Failures header
and manifest.json.
The routes' own messages now raise BatchFileError (app/core/batch.py);
converters raise InvalidInputError (app/converters/base.py) for problems
the user can fix, with a message written for them. Only those two, plus
the existing UnsupportedConversionError, reach the client. Everything else
takes the generic branch, logged with its traceback; that message now
names a next step ("Conversion failed. Verify the file is valid."), since
more files land there. The /api/v1/pdf/* routes already work this way.
Text uploads that aren't UTF-8 — Markdown, CSV, JSON; Excel's default CSV
export on Windows is the common case — go through one helper,
read_utf8_text(), whose message tells the user how to re-save the file:
per file in a batch, and from single /convert as a 400 with
X-FileMorph-Error-Code: invalid_input instead of a generic 500 (a dead end
that API clients would also retry). Without it this fix would have turned
the batch's leaky-but-telling decoder text into a dead end. The helper
keeps line endings, so the CSV readers parse exactly as before, and drops
a leading BOM, which Excel's "CSV UTF-8" (the format the hint recommends)
always writes. The JSON -> CSV "non-empty array" hint, until now visible
only in a batch because it happened to be a ValueError, is an
InvalidInputError now.
Rejected: catching UnicodeDecodeError in the route (binary-format libraries
raise it too, where "not UTF-8 text" would mislead); decoding non-UTF-8
input leniently (no declared charset, so a cp1252 guess can be a silent
false success).
Full suite 1265 green (66 skipped; the WeasyPrint md -> pdf variants run in
CI); ruff + i18n-drift + pip-audit clean.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
scripts/make_testdata_batch_errors.py writes byte-stable fixtures for the manual checklist of the batch error-message fix: a windows-1252 Markdown file and CSV (the "not UTF-8 text" hint), a JSON object that isn't an array (the JSON -> CSV hint) and a small JPEG for a mixed batch. Output goes to the gitignored docs-internal/testdata/batch-errors/; only the script ships, mirroring scripts/make_testdata_format_fixes.py. Verified: two runs are byte-identical; each fixture gives the expected response on /api/v1/convert and /api/v1/convert/batch; ruff clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MrChengLen
force-pushed
the
pr-batch-error-messages
branch
from
September 26, 2026 13:39
9a7b39c to
8c17b01
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What / why
Pre-PR security audit finding (Low, CWE-209):
_do_convert_batchand_do_compress_batchreturnedstr(e)for anyValueError. Library exceptions subclassValueErrortoo —UnicodeDecodeErrorfrom the strict UTF-8 reads in the Markdown/CSV/JSON converters,JSONDecodeError, some Pillow errors — so a non-UTF-8.mdin a batch put'utf-8' codec can't decode byte 0xdc in position 2: invalid continuation byteinto the per-file message, theX-FileMorph-Batch-Failuresheader andmanifest.json.Changes
app/core/batch.py— newBatchFileError, raised by both batch routes for their own messages (filename, size, magic bytes, output cap, target-size format). All existing texts unchanged.app/converters/base.py— newInvalidInputError(a problem the user can fix; message written for them) andread_utf8_text(), which decodes a text upload as UTF-8 or raisesInvalidInputErrortelling the user how to re-save it (Excel: "CSV UTF-8"; editors: Save As → Encoding). Line endings are kept, so the CSV readers parse exactly as before; a leading BOM (Excel's "CSV UTF-8" always writes one) is dropped instead of ending up in the first column name.md → html,md → pdf,csv → json,csv → xlsx,json → csvuseread_utf8_text(). JSON → CSV "must be a non-empty array of objects" is anInvalidInputErrornow.str(e)only forBatchFileError/InvalidInputError(convert also keepsUnsupportedConversionError). Everything else → generic"Conversion failed. Verify the file is valid."/"Compression failed. Verify the file is valid."+logger.exception(the generic text gained the next step because more files land there now)./convertmapsInvalidInputErrorto 400 +X-FileMorph-Error-Code: invalid_input(was a generic 500 — a dead end that API clients would also retry)./api/v1/pdf/*needed no change: it already uses the dedicatedPageSelectionErrorpattern, and_open_readerwraps pypdf errors into a caller-safe message.api-reference.md(400 row),api-usage-guide.md(per-file messages),formats.md(Markdown/CSV/JSON must be UTF-8). CHANGELOG entry under [Unreleased].Behavior changes worth knowing
ValueErrorthat happened to carry readable text now gets the generic message in a batch — as single/convert//compressalways did. Examples: invalid JSON syntax, the API-onlypdf → pdfidentity pair's "Could not read the PDF".X-FileMorph-Error-Codeis already in CORSexpose_headers; no new route or header → network quadruple check N/A.Commits
fix(api): batch errors no longer echo library internals — CWE-209— the fix, docs, CHANGELOG, tests.chore(test): QA fixture generator for the batch error messages—scripts/make_testdata_batch_errors.pyfor the manual checklist (output gitignored, only the script ships; same pattern as fix: six say-vs-do mismatches — PDF/A name, 413 hint, format lists, .htm, env #140).Rebuilt on main 9f639ec: #148 (download-name truncation) touched
convert.py/compress.py/CHANGELOG.md— the route files merge cleanly, my hunks apply unchanged on top of #148; #141 (docker.yml LF) touched onlyCHANGELOG.md. The CHANGELOG keeps every entry.Review gate
commit-review: gitleaks clean, scope-guard clean,security-auditorPASS,code-reviewerapprove; their W1/S1/S3–S5 suggestions are included, the rest are separate follow-ups.Tests
tests/test_conversion_error_messages.py— 15 tests (+2 WeasyPrintmd → pdfvariants that run in Linux CI). On main: 12 fail (leak reproduced for md/csv/json and the injected ValueErrors; BOM kept in the first CSV column), 3 pass (guards: every route-owned message still reaches the client; quoted CSV line breaks survive). With the fix: all pass.🤖 Generated with Claude Code