Skip to content

fix(runtimeConfig): filter localhost URLs from external service config #108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danielheene wants to merge 1 commit into from
Closed

fix(runtimeConfig): filter localhost URLs from external service config #108

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/lib/runtimeConfig/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,26 @@ declare global {
// Dynamic lookup on purpose: `process.env[name]` is never inlined at build time.
const readEnv = (name: string): string | undefined => process.env[name] || undefined

// Localhost URLs in env files are format-valid placeholders (e.g. .env.test).
// Treat them as unset so consumers don't attempt real network calls to them.
const readExternalUrl = (name: string): string | undefined => {

@kilo-code-bot kilo-code-bot Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: No regression test for the new localhost filtering

src/lib/runtimeConfig/index.test.ts already covers readRuntimeConfigFromEnv mapping, but nothing asserts the behavior this PR adds: a localhost/127.0.0.1 URL mapping to undefined while a real URL passes through. A case such as vi.stubEnv('NEXT_PUBLIC_UMAMI_URL', 'http://localhost:3002') with expect(readRuntimeConfigFromEnv().umamiUrl).toBeUndefined() would lock the filter in and matches the repo's regression-test convention for bug fixes.

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

const value = readEnv(name)
if (!value) return undefined
try {
const { hostname } = new URL(value)
if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '::1') return undefined

@kilo-code-bot kilo-code-bot Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: hostname === '::1' never matches — IPv6 loopback URLs are not filtered

The WHATWG URL API serializes IPv6 hosts with brackets: new URL('http://[::1]:3001').hostname is '[::1]', not '::1'. This check is therefore dead code and IPv6-loopback placeholder URLs pass through, even though the commit message claims ::1 is handled. Compare against '[::1]' as well.

Suggested change
if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '::1') return undefined
if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '[::1]') return undefined

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

} catch {
return undefined
}
return value
}

export const readRuntimeConfigFromEnv = (): RuntimeConfig => ({
serverUrl: readEnv('SERVER_URL'),
statusPageUrl: readEnv('STATUS_PAGE_URL'),
umamiUrl: readEnv('NEXT_PUBLIC_UMAMI_URL'),
statusPageUrl: readExternalUrl('STATUS_PAGE_URL'),

@kilo-code-bot kilo-code-bot Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: This does not silence the ServiceStatus heartbeat fetch described in the PR

The PR says this stops ServiceStatus from fetching from localhost:3001, but that fetch uses STATUS_PAGE_HEARTBEAT_URL (http://localhost:3001/api/push/test in .env.test), which is read directly from process.env in src/components/ServiceStatus/ServiceStatus.tsx:25 — not via getRuntimeConfig(). Only the Link href (which never made network calls) is filtered, so the E2E ECONNREFUSED noise from the heartbeat fetch continues. If silencing it is part of the goal, STATUS_PAGE_HEARTBEAT_URL needs the same filtering or a guard in fetchStatus.

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

umamiUrl: readExternalUrl('NEXT_PUBLIC_UMAMI_URL'),

@kilo-code-bot kilo-code-bot Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: umamiUrl can now be undefined, which makes new URL(path, base) throw in consumers

readExternalUrl now maps NEXT_PUBLIC_UMAMI_URL to undefined in E2E, but two consumers pass the value directly as the base of new URL(path, base) with no guard:

  • src/widgets/UmamiWidget/UmamiWidget.data.ts:119 (buildApiUrl)
  • src/lib/fetchers/fetchTrendingBlogPosts.ts:40 (buildApiUrl)

new URL('/api/...', undefined) throws TypeError: Invalid URL because a relative URL requires a base. These calls run before fetcher's try/catch, so fetchWebsite, fetchStats, fetchEvents, fetchPaths, fetchPageViews and fetchTrendingBlogPosts now throw instead of returning null. UmamiWidget.tsx:12 and TrendingBlogPostsBlock/Renderer/Renderer.tsx:26 await them unguarded, so the admin Umami widget and any page using the trending-posts block fail to render in E2E — before this change they degraded gracefully to null (the ECONNREFUSED happened inside the caught fetcher). Guard the missing URL in these buildApiUrl helpers or in the fetch* functions.

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

umamiSiteId: readEnv('NEXT_PUBLIC_UMAMI_SITE_ID'),
iconifyApi: readEnv('NEXT_PUBLIC_ICONIFY_API'),
iconifyApi: readExternalUrl('NEXT_PUBLIC_ICONIFY_API'),
sentryDsn: readEnv('SENTRY_DSN'),
sentryEnvironment: readEnv('SENTRY_ENVIRONMENT'),
})
Expand Down
Loading