StefanandClaude Sonnet 5
071982caec
The invoice line editor already shows <img> pictures pulled from the mfr portal, but the PDF pipeline's HTML-to-MigraDoc converter had no <img> handler, so they never appeared on the actual invoice. InvoiceService now fetches only https://portal.mobilefieldreport.com/mfr/* image sources before rendering (size/type/count capped, failures logged and skipped) and hands the resolved images down through ApplyInvoice into the HTML converter, which renders them inline at their original HTML position and size (px converted to cm, capped against layout overflow). See ADR 0018. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
71 lines
4.6 KiB
Markdown
71 lines
4.6 KiB
Markdown
---
|
||
status: Accepted
|
||
date: 2026-09-10
|
||
applyTo:
|
||
- "Fuchs/code/MigraDocExtensions.cs"
|
||
- "Fuchs/code/FuchsPdf.cs"
|
||
- "Fuchs/Services/InvoiceService.cs"
|
||
- "Fuchs/Services/IInvoiceService.cs"
|
||
- "Fuchs/Services/IPdfService.cs"
|
||
- "Fuchs/Services/FuchsPdfService.cs"
|
||
supersededBy: ""
|
||
---
|
||
|
||
# 0018 — Invoice-text images are fetched at render time from a domain whitelist, not embedded as data URIs
|
||
|
||
## Context
|
||
The invoice line-item description is TinyMCE-authored HTML (`FdsInvoiceData`/`InvoiceBlock.Items`
|
||
item `desc`), and can contain `<img src="https://portal.mobilefieldreport.com/mfr/...">` picture
|
||
links pulled from the mfr ERP — the browser resolves and displays them in the editor. The PDF
|
||
pipeline, however, renders through a hand-rolled HTML→MigraDoc subset converter
|
||
(`MigraDoc.Extensions.Html.HtmlConverter` in `MigraDocExtensions.cs`), which had no `img` handler
|
||
at all: every `<img>`, remote or embedded, was silently dropped, so images the editor showed never
|
||
appeared on the actual invoice.
|
||
|
||
MigraDoc/PdfSharp does not fetch remote URLs itself, and the converter runs synchronously deep
|
||
inside the largely-synchronous `FuchsPdf` static rendering pipeline. Fetching an arbitrary
|
||
user-supplied URL server-side is also an SSRF surface — the invoice text is user/editor-controlled,
|
||
so a naive "download whatever `src` says" would let it reach internal/private network addresses.
|
||
|
||
## Decision
|
||
- Invoice images are inlined into the PDF only when their `src` starts with the literal prefix
|
||
`https://portal.mobilefieldreport.com/mfr/` (`InvoiceService.AllowedInlineImagePrefix`) — the
|
||
only origin the invoice editor's picture links come from. Anything else (any other host, `http://`,
|
||
a different path) is left unrendered, exactly as before this feature — no generic remote fetch.
|
||
- The fetch happens once per invoice render, in `InvoiceService.ResolveInlineImagesAsync`
|
||
(async, off a named `IHttpClientFactory` client `"InvoiceInlineImages"`, 10s timeout), *before*
|
||
the synchronous MigraDoc render starts. `InvoiceService.GenerateInvoicePdf` therefore became
|
||
`GenerateInvoicePdfAsync` — every caller already ran in an async context. Each fetched image is
|
||
capped at 5MB, must report an `image/*` content-type, and the invoice fetches at most 20 distinct
|
||
images; failures (unreachable, wrong type, oversized) are logged and just leave that one image out
|
||
— they never fail the whole render.
|
||
- The resolved `{src → "base64:..."}` map flows down as a plain `IReadOnlyDictionary<string,string>?`
|
||
parameter through `IPdfService.ApplyInvoice` → `FuchsPdf.ApplyInvoice` → `Cell.AddHtml` into the
|
||
`HtmlConverter`, whose new `img` handler only emits an image for a `src` present in that map —
|
||
it does no fetching itself and never sees the network. Image sizing mirrors the HTML: the
|
||
handler reads `width`/`height` (attribute or inline `style`, as TinyMCE's resize handles write
|
||
them) and converts CSS px → cm, capped to 7×10cm so a malformed/huge size can't blow out the
|
||
invoice table layout.
|
||
|
||
## Consequences
|
||
- Any future host serving invoice-editor images must be added to the whitelist deliberately (a
|
||
one-line prefix, or a small allowed-prefixes list if a second origin appears) — this is not
|
||
meant to grow into a generic "fetch any URL" facility.
|
||
- `IInvoiceService.GenerateInvoicePdf` → `GenerateInvoicePdfAsync`, and
|
||
`IInvoiceDraftService.RenderPreview` → `RenderPreviewAsync` (same signature change, same reason)
|
||
are breaking signature changes; anything implementing/mocking these interfaces needs updating.
|
||
- Image src values not covered by the whitelist — including a stray non-mfr remote URL, or a raw
|
||
`data:` URI a user might paste in — still render as nothing, matching the pre-existing (silent)
|
||
drop behaviour. If embedded `data:` images need support later, that's a separate, simpler
|
||
decision (no network fetch involved) and can reuse the same `img` handler contract.
|
||
|
||
## Alternatives considered
|
||
- **Fetch inside the HTML converter itself (sync-over-async `HttpClient.GetAsync(...).Result`)** —
|
||
rejected: violates the project's no-sync-over-async rule and keeps SSRF-relevant network code
|
||
inside a generic, reusable HTML converter instead of a single audited call site.
|
||
- **Fully embed images as `data:` base64 URIs at editor save time** — rejected: doubles storage
|
||
size per invoice edit, and the image is already resolved by the mfr portal itself; re-fetching a
|
||
small number of images once at render time is simpler and keeps the source of truth external.
|
||
- **Allow any remote `http(s)://` host** — rejected on SSRF grounds; every invoice image the editor
|
||
can currently produce is from the mfr portal, so a whitelist costs nothing today.
|