feat(receipt): server-rendered downloadable PDF receipt - #2866
Conversation
GET /receipt/[entryId]/pdf renders the receipt as a branded official PDF (@react-pdf/renderer): Peanut wordmark, issuer line, amount + status, the core receipt rows, and a reference + issue-date footer. It rides the same data path as the page (getHistoryEntry + mapTransactionDataForDrawer), accepts the page's kind/t params, localizes via the app-locale cookie and the app catalogs, and 404s on unknown entries and unresolvable legacy links. Bank identifiers stay masked - the URL is shareable. The Download PDF affordance renders on the public receipt and in-app, gated by the share affordance's conditions narrowed to kinds the receipt page serves (hasReceiptPage, now also backing getReceiptUrl). On web it is a plain download anchor; in Capacitor (static export, no API routes) the click opens the production URL via the Browser sheet. Jest additions: react-pdf's ESM graph in transformIgnorePatterns plus a CJS shim for yoga's ESM-only wasm loader, so the render test produces real PDF bytes. The route test cuts the @/app/actions/clients edge, whose module-scope ranked RPC transport otherwise keeps jest alive.
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Code-analysis diffPainscore total: 7643.75 → 7682.54 (+38.79) 🆕 New findings (43)
…and 23 more. ✅ Resolved (30)
…and 10 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — changes requested
Request changes: PDF localization is not part of the request/cache identity, so shared and native downloads can return the wrong language.
Findings
- MAJOR · src/app/receipt/[entryId]/pdf/route.ts:58 · Key cached PDFs by locale
The PDF bytes vary by theapp-localecookie, but final receipts are stored in the shared CDN cache for an hour under the same URL with noVary. If an es-419 visitor primes a receipt URL, a later en or pt-BR visitor can receive those Spanish bytes. The native path also opens this URL externally while its locale lives in Capacitor Preferences, so a user without a matching browser cookie falls back to English. Put the normalized locale in the PDF URL/cache key and pass the current app locale from the download link (or stop shared caching), then cover cross-locale and native requests.
Checked clean
- Verified the detached worktree HEAD, trusted author, exact head SHA, base ref, and base SHA; the supplied base is not an ancestor of the stacked head, so the three-dot merge-base diff was reviewed.
- Read PR title and description only; no issue comments or review comments were fetched.
- Exact-head CI was green for typecheck, unit, eslint, format, e2e, dependency release-age, analysis, and preview deployment.
- Correctness pass covered receipt entry resolution, status/error paths, PDF model parity, lifecycle timestamps, amount and FX rows, masking, filename generation, public/native affordances, and visibility gates.
- Security pass covered the public route trust boundary, query validation, sensitive-field masking, React PDF text rendering, response headers/cache behavior, dependency/config changes, and the Jest-only Yoga shim.
- Adversarial pass confirmed cross-locale cache priming and the native Preferences-to-external-browser gap; weaker candidates were refuted by existing guards, data shapes, or tests.
- Slop pass covered all 31 changed files, including the stacked receipt-unavailable, friendly-error, OTA cleanup, localization, and test changes; no additional actionable defect survived.
- Official Vercel CDN documentation confirms
s-maxagecaches Function responses and only request headers named byVaryparticipate in the cache key.
Second opinion skipped: openrouter-timeout.
Exact head: 653450b5d805 · Context: repo, web
The PDF bytes vary by locale, but final receipts were CDN-cached for an hour under a locale-less URL - a primed es-419 response would be served to every other locale. And the native path opens the URL in an external browser that has no app cookies, so native users always got English. The affordance now passes ?locale=<current app locale> (next-intl useLocale), making the locale part of the cache key and carrying it into the external browser. The route validates the param against APP_LOCALES; unknown/missing falls back to the app-locale cookie, and those cookie-derived responses are always no-store - only param-keyed responses keep public, s-maxage=3600 (simpler than Vary: Cookie, which CDNs handle inconsistently).
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The prior locale cache-key defect is fixed at this head, and no actionable defect remains in the reviewed change.
Checked clean
- Earlier major finding FIXED: generated PDF links carry a validated locale in the URL; only final receipts with a valid URL locale are CDN-cacheable, while cookie/default fallbacks are no-store.
- Correctness: reviewed the exact follow-up diff plus PDF route failure paths, locale precedence, web/native download behavior, receipt visibility gates, and PDF model/render wiring.
- Security and adversarial cases: checked invalid kinds and entries, backend/render failures, locale cache separation, filename construction, and masking of bank account identifiers.
- Slop: checked the new locale plumbing, predicates, route tests, and download affordance for duplication, dead paths, misleading contracts, and missing edge coverage.
- Exact-head CI succeeded for unit, e2e, typecheck, eslint, format, analyze, and dependency release-age checks; Deploy Preview was still in progress when checked.
- A focused local Jest invocation was unavailable because the detached worktree has no installed Jest binary; the exact-head unit CI check succeeded.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: 6a06d21061f0 · Context: repo
There was a problem hiding this comment.
Chip review — changes requested
The previous locale-cache finding is fixed. One major defect remains: the official PDF corrupts case-sensitive receipt references by uppercasing them.
Findings
- MAJOR · src/app/receipt/[entryId]/pdf/receipt-pdf-model.ts:217 · Preserve case-sensitive receipt IDs
Manteca synthetic receipt IDs are case-sensitive lookup keys, and the current base deliberately keeps transaction.id raw on the web receipt. This model uppercases the official PDF reference; the transfer-ID row at line 181 does the same. For a mixed-case Manteca QR, on-ramp, or off-ramp ID, the route resolves with the raw ID but the downloaded receipt publishes a reference that cannot be used to look the entry up or cross-check it. Keep transaction.id verbatim in both PDF fields and add a mixed-case model test.
Checked clean
- Prior finding FIXED: validated locale is now part of the PDF URL cache key, while missing, unknown, or cookie-derived locale responses are no-store.
- Verified exact head, supplied base and merge base, PR metadata, trusted author, and dev target without reading issue or review comments.
- Exact-head CI completed successfully: typecheck, format, eslint, e2e, unit, dependency-age, analysis, and preview deployment.
- Reviewed route kind resolution, backend and render failure paths, final-state cache policy, locale fallback, PDF model construction, and download visibility gates.
- Reviewed public-route trust boundary, bank-identifier masking, URL/query encoding, CDN cache isolation, filename/header construction, and Capacitor external-open behavior.
- Reviewed dependency and Next.js tracing configuration, localization catalogs, Jest yoga shim, predicate extraction, tests, and lockfile footprint.
- Git history confirms the base fixed this same mixed-case-ID failure on the web receipt by preserving the raw reference.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: 2aadbc91d9ce · Context: repo
Manteca synthetic ids are case-sensitive lookup keys, and the web receipt already publishes transaction.id raw. The PDF uppercased it in both the footer reference and the transfer-id row, so a downloaded receipt for a mixed-case QR, on-ramp or off-ramp id carried a reference that could not be used to look the entry up.
There was a problem hiding this comment.
Chip review — changes requested
Request changes: long transaction identifiers are made indivisible, so normal transaction hashes overflow the PDF receipt row instead of wrapping. The earlier case-sensitive receipt-ID finding is fixed.
Findings
- MAJOR · src/app/receipt/[entryId]/pdf/ReceiptPdfDocument.tsx:19 · Allow long receipt identifiers to wrap
Font.registerHyphenationCallback((word) => [word])disables every break opportunity; it does not make identifiers wrap character-by-character. A standard 66-character EVMtxHashemitted by the PDF model is wider than the 330pt row value, so the transaction-ID row overflows or overlaps instead of wrapping even though the render test still produces valid PDF bytes. Split identifier-like values into character chunks without inserted hyphens, or add local break opportunities to ID/hash fields and cover the resulting layout.
Checked clean
- Earlier [major] case-sensitive receipt ID finding: FIXED — the reference and transfer ID now preserve transaction.id verbatim, with mixed-case regression coverage.
- PDF route data and failure paths, receipt-kind resolution, locale selection, and final-state cache policy.
- Download visibility and browser/Capacitor behavior across receipt-page kinds.
- Bank identifiers remain masked in the public PDF; no new authorization or privilege boundary was introduced.
- Dependency externalization and font output tracing for the deployed route.
- Exact-head CI succeeded: unit, e2e, lint, typecheck, format, analyze, preview, and minimum-release-age checks.
- Official react-pdf v4 hyphenation semantics and the 6.4.0 line-breaking implementation for overlong unbreakable words.
Second opinion skipped: openrouter-timeout.
Exact head: d9c9bcc4cd6c · Context: repo, web
registerHyphenationCallback(word => [word]) returns the word as a single chunk, which gives the layout engine no break opportunity at all — so a 66-char tx hash, wider than the value column, overflowed the row rather than wrapping character-wise as the comment claimed. The render test only checked the bytes, so it stayed green. Two alternatives were tried and rejected against a rendered PDF: breaking via the hyphenation callback wraps but makes react-pdf draw a hyphen at the break, and a hash read off the page must not gain a character; zero-width spaces draw nothing but react-pdf does not treat them as break points, so the value still overflowed. Hard-wrap identifier-like values on an explicit newline instead — verified with pdftotext that the hash now occupies two lines and reassembles byte-exact with no hyphen.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Both earlier major findings are fixed. No actionable correctness, security, or maintainability defect remains at this head.
Findings
-
MAJOR · src/app/receipt/[entryId]/pdf/route.ts:58 · [moonshotai/kimi-k3] Public, expensive PDF render is trivially cache-bypassable (DoS / cost amplification)
GET /receipt/[entryId]/pdf does a full react-pdf render (fontkit shaping, yoga layout, pdfkit + deflate — the test asserts >10KB output and a 30s timeout budget) on every uncached request, with no session check and no rate limiting. The only mitigation isCache-Control: public, s-maxage=3600for final states with a locale param (line 66), but CDN cache keys include the full query string, so?kind=OFFRAMP&locale=en&_=1,&_=2, ... each miss the cache and trigger a fresh render; omittinglocaleor targeting a pending entry isno-storeand always renders (lines 52-58, 66). Any attacker who knows one valid entryId (e.g. their own transactions, or any shared receipt link) can force unbounded server-side renders at serverless cost/latency exhaustion. Fix: put rate limiting / bot protection in front of this route (edge middleware), and/or canonicalize — 301 any request whose query params are not exactly the expected {kind,locale} set — and/or memoize the rendered buffer keyed by entryId+locale+status instead of relying on URL-keyed CDN caching. -
MINOR · src/app/receipt/[entryId]/pdf/route.ts:65 · [moonshotai/kimi-k3] Backend-controlled transaction id interpolated unsanitized into Content-Disposition
Content-Disposition: inline; filename="${model.fileName}"where fileName ispeanut-receipt-${transaction.id}.pdfand transaction.id is data returned by the backend over the network (the model test deliberately keeps arbitrary mixed-case ids like 'MaNtEcA-Qr-7f3B-AbCd' verbatim, confirming ids are not UUID-constrained). An id containing a double quote breaks out of the quoted-string and injects extra disposition tokens into the response header; an id containing CR/LF makes the Headers constructor throw, turning receipt rendering into a 500. Fix: sanitize when building the header value, e.g.model.fileName.replace(/[^\w.-]/g, '_'), or emit an RFC 5987filename*=UTF-8''${encodeURIComponent(...)}parameter.
Checked clean
- Verified the detached-worktree HEAD, supplied base SHA, merge base, trusted author, dev target, PR title, and description; no issue comments or review comments were read.
- Earlier major finding FIXED: receipt IDs remain case-sensitive in the transfer row and footer reference, with a mixed-case regression test.
- Earlier major finding FIXED: long identifier-like row values and footer references are explicitly wrapped without inserting or changing identifier characters, with direct wrapping and real-render coverage.
- Reviewed PDF model parity for amount, status, counterparty, lifecycle dates, exchange rate, fee, transaction ID, transfer ID, memo, and masked bank-account data.
- Reviewed route kind resolution, entry lookup and transformation, locale selection, CDN cache separation, final-state caching, response headers, and 404/500/502 paths.
- Reviewed web and Capacitor download behavior, public/in-app visibility gates, receipt-kind whitelisting, dependency pinning, server font tracing, renderer test shim, and lockfile changes.
- Exact-head CI succeeded for aggregate CI, report, unit, e2e, typecheck, eslint, format, analyze, minimum-release-age, human-authors, preview deployment, and review; manual and ping were skipped.
- Correctness, security, adversarial, and slop passes found no secret exposure, authorization change, unsafe input handling, privilege change, or actionable architecture drift.
Second opinion by moonshotai/kimi-k3: 2 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: ec6032f25733 · Context: repo, github_ci, git-history, peanut-api-ts
The PDF route is public and unauthenticated, and a full react-pdf render (font shaping, layout, deflate) runs per uncached request. A CDN keys on the entire query string, so `?…&_=1`, `&_=2`, … each miss the shared cache and would force a fresh render; cookie-locale and non-final requests are no-store and always rendered. Memoize the rendered bytes per (entryId, kind, locale) for a short window — 60s for final receipts, 10s while one can still change — bounded to 32 entries with LRU eviction, so a burst costs one render and a wide spread of ids cannot grow the map. Also sanitize the id interpolated into the Content-Disposition filename. Ids are arbitrary backend strings (Manteca synthetics are not UUIDs): a double quote would break out of the quoted string and inject disposition tokens, and a CR/LF would make the Headers constructor throw and turn every such receipt into a 500. The human-facing reference still shows it verbatim.
There was a problem hiding this comment.
Chip review — changes requested
One major issue remains: the public-route render cache does not coalesce concurrent misses, so a cache-busted burst still launches a full PDF render per request.
Findings
-
MAJOR · src/app/receipt/[entryId]/pdf/route.ts:93 · Coalesce concurrent PDF renders
On a cold cache key, every request reads the empty map and then awaitsrenderReceiptPdfbefore anything is inserted. For example, 20 simultaneous requests for the same public receipt with different cache-busting query params all reach this line and perform 20 full font/layout/deflate renders; the sequential route test cannot detect that race. This defeats the burst protection and can exhaust a serverless instance. Store an in-flight Promise per key before awaiting it (remove it on rejection), so concurrent callers share one render, then retain the bounded TTL buffer cache. -
MAJOR · src/app/receipt/[entryId]/pdf/route.ts:61 · [moonshotai/kimi-k3] PDF route skips the receipt-kind whitelist, widening the unauthenticated public surface
The route's only check is that resolveReceiptKind(kind, t) returns something, then it fetches and renders any entry. resolveReceiptKind goes through the history strategies registry, which resolves many more kinds than the receipt-page set (e.g. DIRECT_TRANSFER) — including via the legacy numeric ?t= index. So a caller who obtains an entryId (screenshots, referrer logs, shared links of other kinds) can pull a full public PDF receipt for kinds the product explicitly excludes from the public receipt surface. getReceiptUrl and the client-side download gate both restrict to hasReceiptPage (SEND_LINK + FIAT_RAIL_KINDS), but the route itself never applies that predicate, and entryIds are passed through without any authorization. Concrete failure: GET /receipt//pdf?kind=DIRECT_TRANSFER returns application/pdf bytes for an entry whose kind has no receipt page. Fix: after mapTransactionDataForDrawer, check hasReceiptPage(transactionDetails) and return notFound() when false, so the server enforces the same whitelist the links and gates use.
Checked clean
- Exact head/base identity and trusted PR metadata matched the supplied values.
- Earlier long receipt-identifier finding is fixed: hashes and mixed-case synthetic references are hard-wrapped without changing their characters.
- Receipt model mapping, bank-identifier masking, filename sanitization, locale selection, cache headers, visibility gates, and error responses were reviewed.
- All exact-head GitHub gates completed successfully, including unit, typecheck, eslint, format, e2e, analyze, and preview deployment.
- Targeted local Jest execution was unavailable because this detached worktree has no installed Jest binary; exact-head CI supplied the test evidence instead.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: 66fb452042b2 · Context: repo
…he kind whitelist The memo only helped once a render had finished: on a cold key every simultaneous request read the empty map and started its own render, so a burst of cache-busted URLs still ran N full font/layout/deflate passes. One render per key is now in flight at a time and the rest await it; the slot is dropped on rejection so a single failure cannot poison the key. The route also accepted any kind resolveReceiptKind could resolve, which is wider than the set the receipt page publishes (a direct transfer, or a legacy ?t= index, would render). It now gates on hasReceiptPage — the same whitelist the page and the download affordance use — so this public, unauthenticated surface matches the product's.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean. The previous concurrency finding is fixed by per-key in-flight render coalescing with regression coverage; no actionable defect remains.
Checked clean
- Earlier major finding, coalesce concurrent PDF renders: FIXED in the current route and covered by a concurrent-request regression test.
- Exact head, supplied base, trusted PR metadata, and changed-file scope verified without reading issue or review comments.
- PDF route whitelist, backend and render failure paths, locale-aware cache isolation, final-state cache headers, and safe download filenames reviewed.
- PDF model and rendering reviewed for amounts, lifecycle timestamps, counterparty labels, masked bank identifiers, long identifiers, localization, and document metadata.
- Web and Capacitor download behavior plus in-app and public receipt visibility gates reviewed.
- Security and adversarial pass covered the unauthenticated route boundary, header injection, cache separation, render concurrency, and server-only dependency packaging.
- Slop pass covered duplicated receipt-kind policy, dead code, misleading naming, tests, and architecture drift.
- Exact-head CI is green: unit, eslint, e2e, typecheck, format, analyze, Deploy-Preview, and minimum-release-age checks passed.
- Focused local Jest invocation was unavailable because the detached worktree has no installed Jest; the exact-head unit check passed in CI.
Second opinion skipped: openrouter-timeout.
Exact head: b302ea8157b9 · Context: repo, ci
This branch was successfully deployed
1 participant
TASK-19830 part 2 — the downloadable official receipt. Stacked on #2863 (official public receipt); the first two commits here are that PR and will drop out of the diff when it merges.
What this adds
GET /receipt/[entryId]/pdf— a Next route handler rendering a branded, official PDF receipt server-side. It rides the identical data path as the receipt page (getHistoryEntry+mapTransactionDataForDrawer), so PDF and page can never disagree. Accepts the samekind/tparams; 404 for unknown entries and unresolvable legacy links; 502/500 with Sentry capture on BE/render failures; cacheable (1h) only for final states; localized via theapp-localecookie through the same next-intl catalogs.receipt-pdf-model.ts, mirrors the receipt's row-visibility rules) + react-pdf document with brand Montserrat fonts, the wordmark transcribed to react-pdf SVG primitives (recolored for white paper), amount + converted-currency card, detail rows, reference/issued-on footer. Bank identifiers are always masked in the PDF — the URL is shareable, so the in-app guest-claim unmask exception is deliberately not carried over.getReceiptUrlshare one whitelist), and also whenisPublic. On web it's a plaindownloadanchor; in the Capacitor build (where thereceiptdir is stripped from the static export) the click opens the absolute production URL through the existing external-open pattern.Library
@react-pdf/renderer4.6.0 (pinned): newest stable clearing the 14-day supply-chain cooldown (4.7–4.8.x published Aug 23–24; 4.6.0 is 2026-08-08). Nothing PDF-capable existed in the repo (satori/ImageResponse is PNG-only).serverExternalPackages+ font tracing wired innext.config.js; jest gained a transform allowance and a yoga-wasm shim for react-pdf's ESM graph.Verification
application/pdf, 27.6KB,%PDF-1.3, visually inspected (wordmark, ARS conversion card, rows, footer); 404s confirmed for unknown entry / bad kind / unresolvable?t=; legacy?t=10resolves and renders.%PDF-magic through the actual pipeline; affected existing suites green; typecheck clean.Note for reviewers: a route test documents a pre-existing repo-wide jest hazard —
@/app/actions/clientsbuilds a ranked viem fallback client at module scope (pings RPCs on import, 60s re-rank interval), which hangs any node-env suite that reaches it. Mocked in the test, left alone otherwise.Notion: TASK-19830