fix: media upload — statusless errors, remote content upload, controlled validation #13

Merged
jlxq0 merged 5 commits from fix/t_3181e5c4-upload-media into master 2026-10-09 15:02:34 +00:00
Owner

Fleet tasks t_3181e5c4 and t_2dc46427 (both Honoka's cards). This is the one PR for both: t_2dc46427's work fast-forwards this branch from b7e7451. Do not merge without Julian's authorization.

Fork classification: upstreamable. The error mapping, upload validation and listing fix are generic. The AGENTS.md entries are fork-local.

Root cause (reproduced, not assumed)

Reproduced against a real Listmonk v6.1.0 run locally in Docker, through the real MCP interface (streamable HTTP):

  • Path the server can read: the same original SVG uploads fine.
  • Path the server can't see: this is Honoka's case. file_path is read on the server's filesystem, and the fondue pod mounts only /tmp. The client raised ListmonkAPIError(status_code=None) ("File not found"), and the error mapper's 400 <= None raised inside the except handler. That produced exactly '<=' not supported between instances of 'int' and 'NoneType'.

b7e7451 alone fixes the crash and surfaces "File not found", but a remote caller still can't upload. c4e74b6 and 2a67353 make remote upload work.

Changes

  • b7e7451: errors with no HTTP status map to an APIError carrying the client's message.
  • c4e74b6, remote upload: upload_media_file takes optional content_base64 + filename. file_path and title are unchanged for existing callers.
  • c4e74b6, controlled refusals: every refusal is a MediaValidationError raised before any request and reported as ValidationError. That covers missing, inaccessible, not a regular file, unreadable, empty, over max_upload_bytes, unsupported type, bytes that don't match the type, bad base64, and a filename containing a path.
  • c4e74b6, size limit: max_upload_bytes defaults to Listmonk's own 5000 KB. None means no limit and is never compared. The boundary is inclusive.
  • c4e74b6, listing: a missing, null or non-numeric media size shows as unknown instead of breaking.
  • 2a67353, review round 1:
    • the file is opened once with O_NONBLOCK and checked on the descriptor, so it can't be swapped before the read and a FIFO can't block;
    • the read stops at limit + 1 bytes;
    • base64 size is checked by exact decoded length, padding included, before decoding;
    • oversized raw input is refused before whitespace is stripped;
    • a NUL in the path is a controlled error.
  • e034a69: an AGENTS.md wording fix (the exact raw-base64 threshold).
  • AGENTS.md Known Pitfalls:
    • the root cause, and why file_path is server-local;
    • the refusal order and the None limit;
    • what Listmonk v6.1.0 does with media: it ignores title, suffixes duplicate names, has no meta.size, returns data.results, and has no PUT /api/media;
    • the SVG check is a signature check, not a parser.

Tests (tests/test_upload_media.py, 45 tests)

  • Master d39ff97: 35 fail, 6 of them with the original TypeError.
  • b7e7451 alone: 31 fail.
  • This head: 45 pass.
  • Full repo: ruff, mypy, 119 passed, buildcache guard OK.

AC3: real upload, list readback and GET (local Listmonk v6.1.0, not live)

Sent as content_base64 through the MCP client, so the server never saw the path:

Fixture Size Returned URL get_media_list readback GET
tiny original SVG, sha256 cfa1ddbd… 129 B http://127.0.0.1:53659/uploads/t2dc-r2.svg ID: 6 | t2dc-r2.svg, same URL 200 image/svg+xml, 129 B, identical
JPEG, sha256 7e54100b… 225 B http://127.0.0.1:53659/uploads/t2dc-r2.jpg ID: 7, same URL 200 image/jpeg, 225 B, identical
  • file_path route: also uploads; Listmonk suffixed the duplicate name (t2dc-r2_eGxQuj.svg), and the tool reports that name.
  • Refusals against the real server: unseen path, .txt, and fake SVG came back as ValidationError and created no media.
  • No side effects: no campaigns, subscriptions, sends or publication. The fixtures are original and synthetic.

Review (independent, different models)

  • Codex round 1: REWORK, four code findings, all fixed in 2a67353.
  • Codex round 2 on 2a67353: all four FIXED, AC1–AC3 PASS. AC4 failed only on the AGENTS.md threshold wording, fixed in e034a69.
  • gpt-6-luna (commissioned by Alan, read-only) reviewed 2a67353 at 22:02 +08: APPROVE, all criteria met.
  • Not covered by either review: e034a69 is that one AGENTS.md line, 2a67353..e034a69.

Deployment (not done here; Clark)

fondue runs listmonk-mcp:v0.2.1, which has neither fix, so Honoka's mounted tool stays on the old behaviour until a release and Clark's rollout. After rollout, the live check is: Honoka uploads a fixture via content_base64, finds the exact entry in get_media_list, and GETs its URL. That has not been done.

Found, not fixed

rename_media calls PUT /api/media/:id, which Listmonk v6.1.0 doesn't have, so it can't work. That needs its own decision.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MjY9WoGT5SGnSCrYDVdiWJ

Fleet tasks **t_3181e5c4** and **t_2dc46427** (both Honoka's cards). This is the one PR for both: t_2dc46427's work fast-forwards this branch from `b7e7451`. **Do not merge without Julian's authorization.** **Fork classification: upstreamable.** The error mapping, upload validation and listing fix are generic. The AGENTS.md entries are fork-local. ## Root cause (reproduced, not assumed) Reproduced against a real Listmonk v6.1.0 run locally in Docker, through the real MCP interface (streamable HTTP): - **Path the server can read:** the same original SVG uploads fine. - **Path the server can't see:** this is Honoka's case. `file_path` is read on the *server's* filesystem, and the fondue pod mounts only `/tmp`. The client raised `ListmonkAPIError(status_code=None)` ("File not found"), and the error mapper's `400 <= None` raised inside the `except` handler. That produced exactly `'<=' not supported between instances of 'int' and 'NoneType'`. `b7e7451` alone fixes the crash and surfaces "File not found", but a remote caller still can't upload. `c4e74b6` and `2a67353` make remote upload work. ## Changes - **`b7e7451`:** errors with no HTTP status map to an APIError carrying the client's message. - **`c4e74b6`, remote upload:** `upload_media_file` takes optional `content_base64` + `filename`. `file_path` and `title` are unchanged for existing callers. - **`c4e74b6`, controlled refusals:** every refusal is a `MediaValidationError` raised before any request and reported as `ValidationError`. That covers missing, inaccessible, not a regular file, unreadable, empty, over `max_upload_bytes`, unsupported type, bytes that don't match the type, bad base64, and a `filename` containing a path. - **`c4e74b6`, size limit:** `max_upload_bytes` defaults to Listmonk's own 5000 KB. `None` means no limit and is never compared. The boundary is inclusive. - **`c4e74b6`, listing:** a missing, `null` or non-numeric media size shows as `unknown` instead of breaking. - **`2a67353`, review round 1:** - the file is opened once with `O_NONBLOCK` and checked on the descriptor, so it can't be swapped before the read and a FIFO can't block; - the read stops at limit + 1 bytes; - base64 size is checked by exact decoded length, padding included, before decoding; - oversized raw input is refused before whitespace is stripped; - a NUL in the path is a controlled error. - **`e034a69`:** an AGENTS.md wording fix (the exact raw-base64 threshold). - **AGENTS.md Known Pitfalls:** - the root cause, and why `file_path` is server-local; - the refusal order and the `None` limit; - what Listmonk v6.1.0 does with media: it ignores `title`, suffixes duplicate names, has no `meta.size`, returns `data.results`, and has no `PUT /api/media`; - the SVG check is a signature check, not a parser. ## Tests (`tests/test_upload_media.py`, 45 tests) - Master `d39ff97`: 35 fail, 6 of them with the original TypeError. - `b7e7451` alone: 31 fail. - This head: 45 pass. - Full repo: ruff, mypy, 119 passed, buildcache guard OK. ## AC3: real upload, list readback and GET (local Listmonk v6.1.0, not live) Sent as `content_base64` through the MCP client, so the server never saw the path: | Fixture | Size | Returned URL | `get_media_list` readback | GET | |---|---|---|---|---| | tiny original SVG, sha256 `cfa1ddbd…` | 129 B | `http://127.0.0.1:53659/uploads/t2dc-r2.svg` | `ID: 6 \| t2dc-r2.svg`, same URL | 200 `image/svg+xml`, 129 B, identical | | JPEG, sha256 `7e54100b…` | 225 B | `http://127.0.0.1:53659/uploads/t2dc-r2.jpg` | `ID: 7`, same URL | 200 `image/jpeg`, 225 B, identical | - **`file_path` route:** also uploads; Listmonk suffixed the duplicate name (`t2dc-r2_eGxQuj.svg`), and the tool reports that name. - **Refusals against the real server:** unseen path, `.txt`, and fake SVG came back as `ValidationError` and created no media. - **No side effects:** no campaigns, subscriptions, sends or publication. The fixtures are original and synthetic. ## Review (independent, different models) - **Codex round 1:** REWORK, four code findings, all fixed in `2a67353`. - **Codex round 2** on `2a67353`: all four FIXED, AC1–AC3 PASS. AC4 failed only on the AGENTS.md threshold wording, fixed in `e034a69`. - **gpt-6-luna** (commissioned by Alan, read-only) reviewed `2a67353` at 22:02 +08: **APPROVE**, all criteria met. - **Not covered by either review:** `e034a69` is that one AGENTS.md line, `2a67353..e034a69`. ## Deployment (not done here; Clark) fondue runs `listmonk-mcp:v0.2.1`, which has neither fix, so Honoka's mounted tool stays on the old behaviour until a release and Clark's rollout. After rollout, the live check is: Honoka uploads a fixture via `content_base64`, finds the exact entry in `get_media_list`, and GETs its URL. That has not been done. ## Found, not fixed `rename_media` calls `PUT /api/media/:id`, which Listmonk v6.1.0 doesn't have, so it can't work. That needs its own decision. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MjY9WoGT5SGnSCrYDVdiWJ
fix: map statusless ListmonkAPIError instead of crashing
All checks were successful
CI / python (pull_request) Successful in 18s
CI / docker (pull_request) Successful in 16s
b7e7451e87
The client raises ListmonkAPIError with status_code=None when no HTTP
response exists (upload_media on a path the server cannot see, transport
failures). map_http_status_to_exception compared `400 <= None` inside
safe_execute_async's except handler, so upload_media_file surfaced only
"'<=' not supported between instances of 'int' and 'NoneType'" and lost
the real reason. None now maps to APIError carrying the client's message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MjY9WoGT5SGnSCrYDVdiWJ
upload_media_file read file_path on the server's own filesystem. Over
streamable-HTTP the server is a pod, so a caller's local file is "File
not found" there, which used to surface as the int-vs-None TypeError.
Reproduced against a real Listmonk v6.1.0: the same original SVG uploads
when the server can read it.

- content_base64 + filename upload bytes from a caller on another host;
  file_path keeps working unchanged
- every refusal is a MediaValidationError raised before any request and
  reported as ValidationError: missing, inaccessible, not a regular
  file, unreadable, empty, over max_upload_bytes, unsupported type,
  signature mismatch, bad base64, filename with a path
- max_upload_bytes defaults to Listmonk's 5000 KB; None means no limit
  and is never compared; the boundary is inclusive
- media listings tolerate absent/null/non-numeric size ("unknown")

Builds on b7e7451 (statusless error mapping) in PR #13.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MjY9WoGT5SGnSCrYDVdiWJ
Review round 1. Open once with O_NONBLOCK and check the descriptor, so
the file cannot be swapped between check and read and a FIFO cannot
block; bound the read at limit + 1. Measure base64 by its exact decoded
length, padding included, before decoding; refuse raw input over twice
the encoded limit before stripping whitespace. A NUL in file_path is a
controlled error. One regression test per finding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MjY9WoGT5SGnSCrYDVdiWJ
docs: state the exact raw base64 threshold
All checks were successful
CI / python (pull_request) Successful in 20s
CI / docker (pull_request) Successful in 15s
e034a69d50
Review round 2: AGENTS.md said twice the encoded limit; the code allows
2 * encoded_limit + 1024.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MjY9WoGT5SGnSCrYDVdiWJ
jlxq0 changed title from fix: map statusless ListmonkAPIError instead of crashing to fix: media upload — statusless errors, remote content upload, controlled validation 2026-10-09 14:38:10 +00:00
Author
Owner

Third independent review (Codex) on exact head e034a69: APPROVE.

  1. git diff 2a67353 e034a69 touches only AGENTS.md (one line).
  2. The corrected threshold matches client.py:168 exactly: if len(content_base64) > 2 * encoded_limit + 1024
  3. The Known Pitfalls added in d39ff97..e034a69 match the code.
  4. Source, config, fixtures and tests are identical to 2a67353, so the round-2 AC1-AC3 PASS verdicts carry over.

CI on e034a69: CI / python and CI / docker both green. Not merged: this card came from Honoka and needs Julian's authorization.

Third independent review (Codex) on exact head e034a69: **APPROVE**. 1. git diff 2a67353 e034a69 touches only AGENTS.md (one line). 2. The corrected threshold matches client.py:168 exactly: if len(content_base64) > 2 * encoded_limit + 1024 3. The Known Pitfalls added in d39ff97..e034a69 match the code. 4. Source, config, fixtures and tests are identical to 2a67353, so the round-2 AC1-AC3 PASS verdicts carry over. CI on e034a69: CI / python and CI / docker both green. Not merged: this card came from Honoka and needs Julian's authorization.
chore: release v0.2.2
All checks were successful
CI / python (pull_request) Successful in 18s
CI / docker (pull_request) Successful in 15s
cff6f161d9
Version 0.2.1 -> 0.2.2 in pyproject, uv.lock, __version__, the
Dockerfile BUILD_VERSION default and README example, and the buildcache
guard's release-tag fixture, matching the v0.2.1 release. The guard runs
the docker step under refs/tags/v0.2.2 and passes; under v0.2.1 it fails
with 'tag version 0.2.1 does not match pyproject version 0.2.2'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MjY9WoGT5SGnSCrYDVdiWJ
jlxq0 merged commit 07e52021ef into master 2026-10-09 15:02:34 +00:00
Sign in to join this conversation.
No description provided.