Repository navigation
fix(umami): restore /stats rewrite dropped during env-agnostic refactor - #106
Conversation
The async rewrites() block was removed when the compile was made environment-agnostic, but it reads process.env dynamically at server startup (not build time) so it never leaks into the bundle. Without it browser-side /stats/api/send requests hit a missing route, Next.js tries to proxy them to localhost:3002, and ECONNREFUSED errors flood the server log in CI and any environment where Umami is unreachable. Restored with a runtime guard (no-op when NEXT_PUBLIC_UMAMI_URL is unset) and a comment explaining why the dynamic read is safe here.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous Review Summaries (2 snapshots, latest commit 5d954e1)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5d954e1)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit d2fec48)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Reviewed by free · Input: 213.7K · Output: 19.6K · Cached: 179.7K |
…v-leak next.config.ts rewrites() embeds the destination URL into routes-manifest.json at build time, which causes check-env-leak to flag NEXT_PUBLIC_UMAMI_URL in the compiled output. Replace the rewrite with a /stats/[...path] route handler that reads the Umami URL at request time instead.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ): Promise<NextResponse> { | ||
| const umamiUrl = process.env['NEXT_PUBLIC_UMAMI_URL'] | ||
| if (!umamiUrl) { | ||
| return new NextResponse(null, { status: 204 }) |
There was a problem hiding this comment.
WARNING: Silent 204 response hides missing Umami configuration
When NEXT_PUBLIC_UMAMI_URL is not set, the route returns 204 (No Content) instead of 404 or 500. This makes the client think the analytics request succeeded when it was actually dropped. The previous rewrite approach simply didn't create the route (returning []), so clients would get a 404. Consider returning 503 or 404 to surface configuration issues, or at minimum log a warning server-side.
|
|
||
| return new NextResponse(upstream.body, { | ||
| status: upstream.status, | ||
| headers: upstream.headers, |
There was a problem hiding this comment.
SUGGESTION: Filter sensitive headers from upstream response
Passing all upstream headers (upstream.headers) back to the client could leak headers like set-cookie, content-encoding, transfer-encoding, etc. For an analytics proxy, consider filtering to only safe headers (e.g., content-type, cache-control) or explicitly removing sensitive ones.
…ler inlining process.env['NEXT_PUBLIC_UMAMI_URL'] in a server route file is still inlined by the Next.js bundler into server chunks, triggering check-env-leak. Switch to readRuntimeConfigFromEnv().umamiUrl which uses the bracket-notation readEnv() helper that the bundler does not statically inline.
## [1.12.2](v1.12.1...v1.12.2) (2026-10-09) ### Bug Fixes * **umami:** restore /stats rewrite dropped during env-agnostic refactor ([#106](#106)) ([8b412c1](8b412c1))
|
🎉 This PR is included in version 1.12.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This branch was successfully deployed
1 participant
Summary
rewrites()function innext.config.tsthat proxies/stats/:match*→${NEXT_PUBLIC_UMAMI_URL}/:match*fcfa5cb) which removed theasync rewrites()block/stats/api/sendrequests hit a missing route, Next.js tries to proxy tolocalhost:3002(from.env.test), fails withECONNREFUSED, and floods the server logif (!umamiUrl) return []) and a comment explaining whyprocess.env['NEXT_PUBLIC_UMAMI_URL']is safe here (read dynamically at server startup, not inlined at build time, so won't triggercheck-env-leak)Test plan
bun run lintpasses/statsrequests