Skip to content

fix(currency): the browser was calling CoinGecko directly - #815

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/server-only-transitive
Aug 28, 2026
Merged

fix(currency): the browser was calling CoinGecko directly#815
github-actions[bot] merged 1 commit into
mainfrom
fix/server-only-transitive

Conversation

@catomean

Copy link
Copy Markdown
Collaborator

rateSource.server.ts opens with: "Server-side only. Browsers read /api/rates instead, which keeps the rate on one origin (no third-party connect-src) and collapses one upstream call per visitor into one per minute for the whole platform."

That was not true in production. Measured 2026-08-28 on orangecat.ch:

  • api.coingecko.com present in a client chunk on disk (.next/static/chunks/0c7qzb-nbsq51.js)
  • the browser fetched https://api.coingecko.com/api/v3/simple/price directly, 534ms, in the critical path, on page load

So every visitor's IP went to a third party, and "one call per minute for the platform" became one per visitor.

Why the existing guard missed it

serverOnlyModules.test.ts exists to prevent exactly this. It only looked for a .server import inside a file that itself declares 'use client' — and that is not how bundling works. Anything a client component reaches, through however many hops, is compiled into the client bundle, and the files in between carry no directive:

dashboard/bookings/page.tsx  'use client'
  → services/bookings/index.ts        (no directive)
    → services/currency/rates.server.ts
      → services/currency/rateSource.server.ts   → CoinGecko

The walk is transitive now, and the failure prints the whole chain — the import that matters is rarely in the file you'd open first.

Why it lands as a ratchet

It finds 431 client entry points that can reach a .server module, so it is pinned at exactly that rather than demanding zero. Red about unscheduled work is how a gate gets ignored.

Nothing has been retired yet, and the number is not a list to grind down one file at a time: the chains run through shared helpers (config/cat-plans → services/cat/credit-metering → rates.server accounts for a large share), so cutting one edge retires dozens. That refactor belongs in its own change, not smuggled into the commit that first makes the problem visible.

The harm is fixed now

fetchUpstream refuses to call out when it is not running on Node, so even bundled into a client chunk it cannot leak. Deliberately not typeof window: jsdom defines it, so that check would refuse during every component test and report a green suite for a module that never ran.

🤖 Generated with Claude Code

https://claude.ai/code/session_012dpTLxh5GJWeWTF1UEvcD5

rateSource.server.ts opens with "Server-side only. Browsers read
/api/rates instead, which keeps the rate on one origin (no third-party
connect-src) and collapses one upstream call per visitor into one per
minute for the whole platform."

That was not true in production. Measured 2026-08-28 on orangecat.ch:

  api.coingecko.com present in a CLIENT chunk on disk
    /opt/orangecat/app/.next/static/chunks/0c7qzb-nbsq51.js
  browser fetched https://api.coingecko.com/api/v3/simple/price directly
    534ms, in the critical path, on page load

So every visitor's IP went to a third party, and the "one call per
minute for the platform" became one per visitor.

serverOnlyModules.test.ts exists to prevent exactly this and did not,
because it only looked for a `.server` import inside a file that itself
declares 'use client'. That is not how bundling works: anything a client
component reaches, through however many hops, is compiled into the client
bundle, and the files in between carry no directive. The real chain was
three hops and every file after the first looked innocent:

  dashboard/bookings/page.tsx  'use client'
    → services/bookings/index.ts        (no directive)
      → services/currency/rates.server.ts
        → services/currency/rateSource.server.ts   → CoinGecko

The walk is transitive now and the failure prints the whole chain, since
the import that matters is rarely in the file you would open first.

It finds 431 client entry points that can reach a .server module, so it
lands as a ratchet at exactly that number rather than a demand for zero
— red about unscheduled work is how a gate gets ignored. Nothing has
been retired yet and the number is not a list to grind down one file at
a time: the chains run through shared helpers (config/cat-plans →
services/cat/credit-metering → rates.server accounts for a large share),
so cutting one edge retires dozens. That refactor belongs in its own
change, not smuggled into the commit that first makes the problem
visible.

The concrete harm is fixed now and defensively: fetchUpstream refuses to
call out when it is not running on Node, so even bundled into a client
chunk it cannot leak. Deliberately not `typeof window` — jsdom defines
it, so that check would refuse during every component test and report a
green suite for a module that never ran.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012dpTLxh5GJWeWTF1UEvcD5
@github-actions
github-actions Bot merged commit 4886573 into main Aug 28, 2026
6 checks passed
@github-actions
github-actions Bot deleted the fix/server-only-transitive branch August 28, 2026 16:19
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