From cafff003a336716f6124ef185f0b4b741463abe1 Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 28 Jul 2026 20:21:09 +0200 Subject: [PATCH] refactor(server): move instance-URL resolution to app-config --- server/src/app-config/README.md | 12 +++ server/src/app-config/app-url.ts | 47 +++++++++++ server/src/app-config/index.ts | 1 + server/src/app-config/parsers.ts | 2 +- server/src/index.ts | 3 +- server/src/mcp/index.ts | 2 +- server/src/mcp/oauthProvider.ts | 2 +- server/src/nest/auth/auth.service.ts | 3 +- server/src/nest/oauth/oauth.service.ts | 2 +- server/src/nest/oidc/oidc.service.ts | 2 +- server/src/nest/platform/platform.routes.ts | 6 +- .../src/nest/plugins/plugin-oauth.service.ts | 2 +- server/src/services/mapsService.ts | 2 +- server/src/services/notificationService.ts | 3 +- server/src/services/notifications.ts | 42 +--------- server/src/services/oauthService.ts | 2 +- server/src/services/transitService.ts | 2 +- server/src/services/webauthnConfig.ts | 2 +- server/tests/e2e/auth.e2e.test.ts | 6 +- server/tests/e2e/oauth.e2e.test.ts | 5 +- server/tests/e2e/oidc.e2e.test.ts | 5 +- server/tests/integration/oauth.test.ts | 4 +- server/tests/unit/app-config/app-url.test.ts | 82 +++++++++++++++++++ server/tests/unit/nest/oauth.service.test.ts | 7 +- server/tests/unit/nest/oidc.service.test.ts | 7 +- .../unit/nest/platform.controller.test.ts | 5 +- .../unit/plugins/plugin-oauth.service.test.ts | 5 +- .../unit/services/transitService.test.ts | 5 +- .../unit/services/webauthnConfig.test.ts | 7 +- 29 files changed, 202 insertions(+), 73 deletions(-) create mode 100644 server/src/app-config/app-url.ts create mode 100644 server/tests/unit/app-config/app-url.test.ts diff --git a/server/src/app-config/README.md b/server/src/app-config/README.md index d3fcf85fe..718db3038 100644 --- a/server/src/app-config/README.md +++ b/server/src/app-config/README.md @@ -9,6 +9,7 @@ the pre-init Express middleware. env.schema.ts Zod catalog of the whole env surface; fail-fast at boot derive.ts pure (raw env) → typed namespace functions, exact per-site coercions env.ts readEnv() live accessor + validateEnvAtBoot() +app-url.ts getAppUrl()/getMcpSafeUrl() instance base-URL resolution boot-validate.ts side-effect import used by index.ts (validates before other modules load) parsers.ts shared coercion helpers (boolTrueLoose, numberOr, csvList, …) ``` @@ -43,6 +44,17 @@ tokens, `RuntimeEnvService`) and consumes the SAME derive functions. rate limits, …) keep freeze-at-import timing, merely sourcing the value from `readEnv()` at module top. Request-time reads stay request-time. +## app-url.ts — instance base-URL resolution (moved 2026-07-28) + +`getAppUrl()` (APP_URL → first ALLOWED_ORIGINS entry → `http://localhost:PORT`, +each candidate URL-validated, ALL trailing slashes stripped) and +`getMcpSafeUrl()` (same, sanitized to HTTPS/localhost/127.0.0.1 for the MCP +SDK's issuer check) moved here **verbatim** from `services/notifications.ts` +(audit findings `auth-2`/`notifications-1`/`admin-3`/`mcp-1`). They follow +invariant 1: `readEnv()` per call, live, never cached. The invalid-URL silent +fallthrough and the strip-ALL-slashes quirk (see `parsers.ts` +`stripTrailingSlashes`) are parity-pinned — do not "fix" them here. + ## Classification: boot-stable vs runtime-toggled **Boot-stable** (frozen at app/module creation; snapshot `registerAs`/`ConfigType` diff --git a/server/src/app-config/app-url.ts b/server/src/app-config/app-url.ts new file mode 100644 index 000000000..f91f5038e --- /dev/null +++ b/server/src/app-config/app-url.ts @@ -0,0 +1,47 @@ +import { readEnv } from './env'; + +/** + * Instance base-URL resolution: APP_URL → first ALLOWED_ORIGINS entry → + * http://localhost:{PORT}. Moved verbatim from services/notifications.ts + * (2026-07-28, findings auth-2/notifications-1/admin-3/mcp-1) — the resolution + * chain, trailing-slash strip and invalid-URL fallthrough are parity-pinned. + * Live reads: readEnv() per call, never cached. + */ +export function getAppUrl(): string { + const appUrl = readEnv().app.appUrl; + if (appUrl) { + try { + const _ = new URL(appUrl); + return appUrl.replace(/\/+$/, ''); + } catch (_ignored) {} + } + const origins = readEnv().http.allowedOriginsRaw; + if (origins) { + const first = origins.split(',')[0]?.trim(); + if (first) { + try { + const _ = new URL(first); + return first.replace(/\/+$/, ''); + } catch (_ignored) {} + } + } + const port = readEnv().app.port; + return `http://localhost:${port}`; +} + +/** Returns a URL guaranteed to satisfy the MCP SDK's issuer requirements (HTTPS or localhost). + * Falls back to http://localhost:{PORT} when APP_URL/ALLOWED_ORIGINS use a non-HTTPS, non-localhost scheme + * that would cause checkIssuerUrl to throw "Issuer URL must be HTTPS". */ +export function getMcpSafeUrl(): string { + const candidate = getAppUrl(); + try { + const u = new URL(candidate); + if (u.protocol === 'https:' || u.hostname === 'localhost' || u.hostname === '127.0.0.1') { + return candidate; + } + } catch { + // candidate was somehow invalid — fall through to localhost + } + const port = readEnv().app.port; + return `http://localhost:${port}`; +} diff --git a/server/src/app-config/index.ts b/server/src/app-config/index.ts index d7ecf0dfb..5da92b321 100644 --- a/server/src/app-config/index.ts +++ b/server/src/app-config/index.ts @@ -1,4 +1,5 @@ export { readEnv, validateEnvAtBoot } from './env'; +export { getAppUrl, getMcpSafeUrl } from './app-url'; export type { AppEnv, RawEnv } from './env'; export { deriveAll, diff --git a/server/src/app-config/parsers.ts b/server/src/app-config/parsers.ts index 5c7201363..aa8250cf3 100644 --- a/server/src/app-config/parsers.ts +++ b/server/src/app-config/parsers.ts @@ -61,7 +61,7 @@ export function csvListFiltered(raw: string | undefined): string[] | null { : null; } -/** Strip ALL trailing slashes (`/\/+$/`) — notifications.getAppUrl / TRANSIT_API_URL variant. */ +/** Strip ALL trailing slashes (`/\/+$/`) — app-url.getAppUrl / TRANSIT_API_URL variant. */ export function stripTrailingSlashes(value: string): string { return value.replace(/\/+$/, ''); } diff --git a/server/src/index.ts b/server/src/index.ts index e48fbaeb6..97b0bff2c 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -24,8 +24,7 @@ const tmpDir = path.join(__dirname, '../data/tmp'); }); import * as scheduler from './scheduler'; -import { readEnv } from './app-config'; -import { getAppUrl, getMcpSafeUrl } from './services/notifications'; +import { getAppUrl, getMcpSafeUrl, readEnv } from './app-config'; const PORT = readEnv().app.port; const HOST = readEnv().app.host; diff --git a/server/src/mcp/index.ts b/server/src/mcp/index.ts index ebbd78b32..487cc80e6 100644 --- a/server/src/mcp/index.ts +++ b/server/src/mcp/index.ts @@ -12,7 +12,7 @@ import { registerTools } from './tools'; import { McpSession, sessions, revokeUserSessions, revokeUserSessionsForClient, evictOldestSessionForUser } from './sessionManager'; import { readEnv } from '../app-config'; import { writeAudit, getClientIp } from '../nest/audit/audit.bridge'; -import { getMcpSafeUrl } from '../services/notifications'; +import { getMcpSafeUrl } from '../app-config'; export { revokeUserSessions, revokeUserSessionsForClient }; diff --git a/server/src/mcp/oauthProvider.ts b/server/src/mcp/oauthProvider.ts index 87e9bdffa..e49edc0d0 100644 --- a/server/src/mcp/oauthProvider.ts +++ b/server/src/mcp/oauthProvider.ts @@ -16,7 +16,7 @@ import { getUserByAccessToken, } from '../services/oauthService'; import { ALL_SCOPES } from './scopes'; -import { getMcpSafeUrl } from '../services/notifications'; +import { getMcpSafeUrl } from '../app-config'; import { writeAudit } from '../nest/audit/audit.bridge'; // --------------------------------------------------------------------------- diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index 049106111..aa954620b 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -2,7 +2,8 @@ import { Injectable } from '@nestjs/common'; import type { Request, Response } from 'express'; import * as auth from '../../services/authService'; import { setAuthCookie, clearAuthCookie } from '../../services/cookie'; -import { sendPasswordResetEmail, getAppUrl } from '../../services/notifications'; +import { sendPasswordResetEmail } from '../../services/notifications'; +import { getAppUrl } from '../../app-config'; import type { User } from '../../types'; /** diff --git a/server/src/nest/oauth/oauth.service.ts b/server/src/nest/oauth/oauth.service.ts index 329b494ab..281d3c3a1 100644 --- a/server/src/nest/oauth/oauth.service.ts +++ b/server/src/nest/oauth/oauth.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common'; import * as oauth from '../../services/oauthService'; import { isAddonEnabled } from '../../services/adminService'; import { ADDON_IDS } from '../../addons'; -import { getMcpSafeUrl } from '../../services/notifications'; +import { getMcpSafeUrl } from '../../app-config'; /** * Thin Nest wrapper around the existing OAuth 2.1 service. The grant handling, diff --git a/server/src/nest/oidc/oidc.service.ts b/server/src/nest/oidc/oidc.service.ts index 726a3e182..d9ad28c51 100644 --- a/server/src/nest/oidc/oidc.service.ts +++ b/server/src/nest/oidc/oidc.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import type { Request, Response } from 'express'; import * as oidc from '../../services/oidcService'; -import { getAppUrl } from '../../services/notifications'; +import { getAppUrl } from '../../app-config'; import { resolveAuthToggles } from '../../services/authService'; import { setAuthCookie } from '../../services/cookie'; diff --git a/server/src/nest/platform/platform.routes.ts b/server/src/nest/platform/platform.routes.ts index 9f84e7e5f..da83a1ba6 100644 --- a/server/src/nest/platform/platform.routes.ts +++ b/server/src/nest/platform/platform.routes.ts @@ -14,7 +14,7 @@ import { mcpAuthMetadataRouter } from '@modelcontextprotocol/sdk/server/auth/rou import { authorizationHandler } from '@modelcontextprotocol/sdk/server/auth/handlers/authorize'; import { clientRegistrationHandler } from '@modelcontextprotocol/sdk/server/auth/handlers/register'; import type { OAuthMetadata } from '@modelcontextprotocol/sdk/shared/auth'; -import { getMcpSafeUrl } from '../../services/notifications'; +import { getMcpSafeUrl } from '../../app-config'; // Platform / transport routes extracted verbatim from createApp() (app.ts) so they can be // mounted on either the legacy Express app or the NestJS Express instance (strangler A6/A8). @@ -126,8 +126,8 @@ export function applyPlatformTransport(app: express.Application): void { next(); }; - // SDK metadata router — built lazily on first request so getAppUrl() (which queries the DB) - // is not called at createApp() time, before test tables have been created. + // SDK metadata router — built lazily on first request so the issuer URL is + // resolved from the live env, not frozen at createApp() time. // mcpAuthMetadataRouter serves: // /.well-known/oauth-authorization-server — RFC 8414 AS metadata // /.well-known/oauth-protected-resource/mcp — RFC 9728 path-based PRM (fixes issue #959 bug 1) diff --git a/server/src/nest/plugins/plugin-oauth.service.ts b/server/src/nest/plugins/plugin-oauth.service.ts index dafad14f6..bba838a46 100644 --- a/server/src/nest/plugins/plugin-oauth.service.ts +++ b/server/src/nest/plugins/plugin-oauth.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common'; import crypto from 'node:crypto'; import { DatabaseService } from '../database/database.service'; import { encrypt_api_key, decrypt_api_key } from '../../services/apiKeyCrypto'; -import { getAppUrl } from '../../services/notifications'; +import { getAppUrl } from '../../app-config'; import { isPrivateIp } from './install/safe-fetch'; import { safeFetchLlm } from '../../utils/ssrfGuard'; diff --git a/server/src/services/mapsService.ts b/server/src/services/mapsService.ts index e5f243019..f2abe6103 100644 --- a/server/src/services/mapsService.ts +++ b/server/src/services/mapsService.ts @@ -2,7 +2,7 @@ import { readEnv } from '../app-config'; import { db } from '../db/database'; import { safeFetchFollow, SsrfBlockedError } from '../utils/ssrfGuard'; import { decrypt_api_key } from './apiKeyCrypto'; -import { getAppUrl } from './notifications'; +import { getAppUrl } from '../app-config'; // ── Photo cache (disk-backed) ──────────────────────────────────────────────── import * as placePhotoCache from './placePhotoCache'; diff --git a/server/src/services/notificationService.ts b/server/src/services/notificationService.ts index b43d94ffa..193e90869 100644 --- a/server/src/services/notificationService.ts +++ b/server/src/services/notificationService.ts @@ -9,7 +9,8 @@ import { ADMIN_SCOPED_EVENTS, type NotifEventType, } from './notificationPreferencesService'; -import { getEventText, getUserLanguage, getAppUrl } from './notifications'; +import { getEventText, getUserLanguage } from './notifications'; +import { getAppUrl } from '../app-config'; import { listChannels, type ChannelMessage, type ExternalChannel } from './notifications/channelRegistry'; import { resolveRecipients, diff --git a/server/src/services/notifications.ts b/server/src/services/notifications.ts index c484a988a..d31dc5313 100644 --- a/server/src/services/notifications.ts +++ b/server/src/services/notifications.ts @@ -1,4 +1,4 @@ -import { readEnv } from '../app-config'; +import { getAppUrl, readEnv } from '../app-config'; import { db } from '../db/database'; import { checkSsrf, createPinnedDispatcher } from '../utils/ssrfGuard'; import { decrypt_api_key } from './apiKeyCrypto'; @@ -68,46 +68,6 @@ function getSmtpConfig(): SmtpConfig | null { }; } -// Exported for use by notificationService -export function getAppUrl(): string { - const appUrl = readEnv().app.appUrl; - if (appUrl) { - try { - const _ = new URL(appUrl); - return appUrl.replace(/\/+$/, ''); - } catch (_ignored) {} - } - const origins = readEnv().http.allowedOriginsRaw; - if (origins) { - const first = origins.split(',')[0]?.trim(); - if (first) { - try { - const _ = new URL(first); - return first.replace(/\/+$/, ''); - } catch (_ignored) {} - } - } - const port = readEnv().app.port; - return `http://localhost:${port}`; -} - -/** Returns a URL guaranteed to satisfy the MCP SDK's issuer requirements (HTTPS or localhost). - * Falls back to http://localhost:{PORT} when APP_URL/ALLOWED_ORIGINS use a non-HTTPS, non-localhost scheme - * that would cause checkIssuerUrl to throw "Issuer URL must be HTTPS". */ -export function getMcpSafeUrl(): string { - const candidate = getAppUrl(); - try { - const u = new URL(candidate); - if (u.protocol === 'https:' || u.hostname === 'localhost' || u.hostname === '127.0.0.1') { - return candidate; - } - } catch { - // candidate was somehow invalid — fall through to localhost - } - const port = readEnv().app.port; - return `http://localhost:${port}`; -} - /** Is SMTP configured at the instance level? (Independent of any one user's address.) */ export function isSmtpConfigured(): boolean { return !!(readEnv().smtp.host || getAppSetting('smtp_host')); diff --git a/server/src/services/oauthService.ts b/server/src/services/oauthService.ts index bf46a0efa..4080677d1 100644 --- a/server/src/services/oauthService.ts +++ b/server/src/services/oauthService.ts @@ -6,7 +6,7 @@ import { ADDON_IDS } from '../addons'; import { User } from '../types'; import { writeAudit, logWarn } from '../nest/audit/audit.bridge'; import { revokeUserSessionsForClient } from '../mcp/sessionManager'; -import { getMcpSafeUrl } from './notifications'; +import { getMcpSafeUrl } from '../app-config'; // --------------------------------------------------------------------------- // Constants diff --git a/server/src/services/transitService.ts b/server/src/services/transitService.ts index 905c5f35f..ee19afd28 100644 --- a/server/src/services/transitService.ts +++ b/server/src/services/transitService.ts @@ -1,6 +1,6 @@ import { readEnv } from '../app-config'; import { buildUserAgent } from './mapsService'; -import { getAppUrl } from './notifications'; +import { getAppUrl } from '../app-config'; /** * Public transit routing (#1065) backed by Transitous (api.transitous.org), the diff --git a/server/src/services/webauthnConfig.ts b/server/src/services/webauthnConfig.ts index 2c4271833..a7f68c969 100644 --- a/server/src/services/webauthnConfig.ts +++ b/server/src/services/webauthnConfig.ts @@ -1,6 +1,6 @@ import { readEnv } from '../app-config'; import { db } from '../db/database'; -import { getAppUrl } from './notifications'; +import { getAppUrl } from '../app-config'; /** * Resolves the WebAuthn Relying Party ID + allowed origins for this deployment. diff --git a/server/tests/e2e/auth.e2e.test.ts b/server/tests/e2e/auth.e2e.test.ts index 2d3d58bc3..959eb7d31 100644 --- a/server/tests/e2e/auth.e2e.test.ts +++ b/server/tests/e2e/auth.e2e.test.ts @@ -32,7 +32,11 @@ vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: ( // The audit domain is DI-native now: writeAudit runs for real against the temp // db's audit_log table; only the file logger is silenced. vi.mock('../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logDebug: vi.fn(), logError: vi.fn(), logWarn: vi.fn() })); -vi.mock('../../src/services/notifications', () => ({ getAppUrl: () => 'https://x', sendPasswordResetEmail: vi.fn().mockResolvedValue({ delivered: true }) })); +vi.mock('../../src/services/notifications', () => ({ sendPasswordResetEmail: vi.fn().mockResolvedValue({ delivered: true }) })); +vi.mock('../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAppUrl: () => 'https://x' }; +}); const { authSvc } = vi.hoisted(() => ({ authSvc: { diff --git a/server/tests/e2e/oauth.e2e.test.ts b/server/tests/e2e/oauth.e2e.test.ts index 4465a611e..88c1d3944 100644 --- a/server/tests/e2e/oauth.e2e.test.ts +++ b/server/tests/e2e/oauth.e2e.test.ts @@ -31,7 +31,10 @@ vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: ( // The audit domain is DI-native now: writeAudit runs for real against the temp // db's audit_log table; only the file logger is silenced. vi.mock('../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logDebug: vi.fn(), logError: vi.fn(), logWarn: vi.fn() })); -vi.mock('../../src/services/notifications', () => ({ getMcpSafeUrl: () => 'https://app' })); +vi.mock('../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getMcpSafeUrl: () => 'https://app' }; +}); const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn(() => true) })); vi.mock('../../src/services/adminService', () => ({ isAddonEnabled })); diff --git a/server/tests/e2e/oidc.e2e.test.ts b/server/tests/e2e/oidc.e2e.test.ts index fa60581f2..0c2daf0fe 100644 --- a/server/tests/e2e/oidc.e2e.test.ts +++ b/server/tests/e2e/oidc.e2e.test.ts @@ -10,7 +10,10 @@ import cookieParser from 'cookie-parser'; import type { Server } from 'http'; import { Test } from '@nestjs/testing'; -vi.mock('../../src/services/notifications', () => ({ getAppUrl: () => 'https://app' })); +vi.mock('../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAppUrl: () => 'https://app' }; +}); const { toggles } = vi.hoisted(() => ({ toggles: { oidc_login: true } })); vi.mock('../../src/services/authService', () => ({ resolveAuthToggles: () => toggles })); diff --git a/server/tests/integration/oauth.test.ts b/server/tests/integration/oauth.test.ts index 0736d6c04..50e804287 100644 --- a/server/tests/integration/oauth.test.ts +++ b/server/tests/integration/oauth.test.ts @@ -53,8 +53,8 @@ vi.mock('../../src/services/adminService', async (importOriginal) => { return { ...actual, isAddonEnabled: isAddonEnabledMock }; }); -vi.mock('../../src/services/notifications', async (importOriginal) => { - const actual = await importOriginal(); +vi.mock('../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); return { ...actual, getMcpSafeUrl: () => 'https://trek.example.com' }; }); diff --git a/server/tests/unit/app-config/app-url.test.ts b/server/tests/unit/app-config/app-url.test.ts new file mode 100644 index 000000000..b525ca239 --- /dev/null +++ b/server/tests/unit/app-config/app-url.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; + +import { getAppUrl, getMcpSafeUrl } from '../../../src/app-config/app-url'; + +// These tests PIN the exact behavior moved verbatim from +// services/notifications.ts (2026-07-28): the APP_URL → ALLOWED_ORIGINS → +// localhost fallback chain, the strip-ALL-trailing-slashes quirk, the +// invalid-URL silent fallthrough, and getMcpSafeUrl's HTTPS/localhost issuer +// sanitization. If one fails after an edit, the edit changed runtime +// behavior — fix the edit, not the test (parity is law). + +const KEYS = ['APP_URL', 'ALLOWED_ORIGINS', 'PORT'] as const; +const saved: Record = {}; + +beforeEach(() => { + for (const k of KEYS) { + saved[k] = process.env[k]; + delete process.env[k]; + } +}); + +afterEach(() => { + for (const k of KEYS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } +}); + +describe('getAppUrl', () => { + it('returns APP_URL with ALL trailing slashes stripped', () => { + process.env.APP_URL = 'https://trek.example.com///'; + expect(getAppUrl()).toBe('https://trek.example.com'); + }); + + it('falls through an invalid APP_URL to the first ALLOWED_ORIGINS entry', () => { + process.env.APP_URL = 'not a url'; + process.env.ALLOWED_ORIGINS = 'https://a.example/, https://b.example'; + expect(getAppUrl()).toBe('https://a.example'); + }); + + it('uses the first ALLOWED_ORIGINS entry (trimmed, slashes stripped) when APP_URL is unset', () => { + process.env.ALLOWED_ORIGINS = ' https://origin.example// ,https://other.example'; + expect(getAppUrl()).toBe('https://origin.example'); + }); + + it('falls back to http://localhost:{PORT} when neither is usable', () => { + process.env.ALLOWED_ORIGINS = 'not-a-url'; + process.env.PORT = '4123'; + expect(getAppUrl()).toBe('http://localhost:4123'); + }); + + it('defaults to port 3001 with no env at all', () => { + expect(getAppUrl()).toBe('http://localhost:3001'); + }); + + it('reads the env live — no caching across calls', () => { + process.env.APP_URL = 'https://first.example'; + expect(getAppUrl()).toBe('https://first.example'); + process.env.APP_URL = 'https://second.example'; + expect(getAppUrl()).toBe('https://second.example'); + }); +}); + +describe('getMcpSafeUrl', () => { + it('passes through HTTPS URLs', () => { + process.env.APP_URL = 'https://trek.example.com'; + expect(getMcpSafeUrl()).toBe('https://trek.example.com'); + }); + + it('passes through http://localhost and http://127.0.0.1', () => { + process.env.APP_URL = 'http://localhost:8080'; + expect(getMcpSafeUrl()).toBe('http://localhost:8080'); + process.env.APP_URL = 'http://127.0.0.1:8080'; + expect(getMcpSafeUrl()).toBe('http://127.0.0.1:8080'); + }); + + it('sanitizes non-HTTPS, non-localhost URLs to http://localhost:{PORT}', () => { + process.env.APP_URL = 'http://trek.internal.lan'; + process.env.PORT = '3005'; + expect(getMcpSafeUrl()).toBe('http://localhost:3005'); + }); +}); diff --git a/server/tests/unit/nest/oauth.service.test.ts b/server/tests/unit/nest/oauth.service.test.ts index 3b1447b8d..66bbdd3c7 100644 --- a/server/tests/unit/nest/oauth.service.test.ts +++ b/server/tests/unit/nest/oauth.service.test.ts @@ -29,7 +29,10 @@ const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn() })); vi.mock('../../../src/services/adminService', () => ({ isAddonEnabled })); const { getMcpSafeUrl } = vi.hoisted(() => ({ getMcpSafeUrl: vi.fn() })); -vi.mock('../../../src/services/notifications', () => ({ getMcpSafeUrl })); +vi.mock('../../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getMcpSafeUrl }; +}); import { OauthService } from '../../../src/nest/oauth/oauth.service'; import { ADDON_IDS } from '../../../src/addons'; @@ -47,7 +50,7 @@ describe('OauthService', () => { expect(svc().mcpEnabled()).toBe(false); }); - it('mcpSafeUrl forwards to the notifications helper', () => { + it('mcpSafeUrl forwards to the app-config helper', () => { getMcpSafeUrl.mockReturnValue('https://safe'); expect(svc().mcpSafeUrl()).toBe('https://safe'); expect(getMcpSafeUrl).toHaveBeenCalled(); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index ae198a1fb..c1ee693be 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -24,7 +24,10 @@ const { oidc } = vi.hoisted(() => ({ vi.mock('../../../src/services/oidcService', () => oidc); const { getAppUrl } = vi.hoisted(() => ({ getAppUrl: vi.fn() })); -vi.mock('../../../src/services/notifications', () => ({ getAppUrl })); +vi.mock('../../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAppUrl }; +}); const { resolveAuthToggles } = vi.hoisted(() => ({ resolveAuthToggles: vi.fn() })); vi.mock('../../../src/services/authService', () => ({ resolveAuthToggles })); @@ -54,7 +57,7 @@ describe('OidcService', () => { expect(s.getOidcConfig()).toBe(cfg); }); - it('getAppUrl delegates to notifications.getAppUrl', () => { + it('getAppUrl delegates to app-config getAppUrl', () => { getAppUrl.mockReturnValue('https://app'); expect(s.getAppUrl()).toBe('https://app'); }); diff --git a/server/tests/unit/nest/platform.controller.test.ts b/server/tests/unit/nest/platform.controller.test.ts index 73f522879..d5417857e 100644 --- a/server/tests/unit/nest/platform.controller.test.ts +++ b/server/tests/unit/nest/platform.controller.test.ts @@ -21,7 +21,10 @@ vi.mock('../../../src/db/database', () => ({ db: { prepare: h.dbPrepare } })); vi.mock('../../../src/mcp', () => ({ mcpHandler: h.mcpHandler })); vi.mock('../../../src/mcp/oauthProvider', () => ({ trekOAuthProvider: {}, trekClientsStore: {} })); vi.mock('../../../src/services/adminService', () => ({ isAddonEnabled: h.isAddonEnabled })); -vi.mock('../../../src/services/notifications', () => ({ getMcpSafeUrl: h.getMcpSafeUrl })); +vi.mock('../../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getMcpSafeUrl: h.getMcpSafeUrl }; +}); // SDK router/handler factories return distinct tagged middleware so we never hit // real new URL(...) wiring during registration. diff --git a/server/tests/unit/plugins/plugin-oauth.service.test.ts b/server/tests/unit/plugins/plugin-oauth.service.test.ts index 79763dba9..267d00401 100644 --- a/server/tests/unit/plugins/plugin-oauth.service.test.ts +++ b/server/tests/unit/plugins/plugin-oauth.service.test.ts @@ -10,7 +10,10 @@ vi.mock('../../../src/services/apiKeyCrypto', () => ({ encrypt_api_key: (v: unknown) => (typeof v === 'string' ? `enc:${v}` : v), decrypt_api_key: (v: unknown) => (typeof v === 'string' && v.startsWith('enc:') ? v.slice(4) : v), })); -vi.mock('../../../src/services/notifications', () => ({ getAppUrl: () => 'https://trek.example' })); +vi.mock('../../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAppUrl: () => 'https://trek.example' }; +}); const { getDb } = vi.hoisted(() => ({ getDb: { current: null as unknown } })); vi.mock('../../../src/db/database', () => ({ get db() { return getDb.current; } })); diff --git a/server/tests/unit/services/transitService.test.ts b/server/tests/unit/services/transitService.test.ts index 6864f5737..0100afa2c 100644 --- a/server/tests/unit/services/transitService.test.ts +++ b/server/tests/unit/services/transitService.test.ts @@ -7,7 +7,10 @@ import { deriveTransitStats, geocode, plan, type TransitLeg } from '../../../src import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -vi.mock('../../../src/services/notifications', () => ({ getAppUrl: () => 'https://trek.example.com' })); +vi.mock('../../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAppUrl: () => 'https://trek.example.com' }; +}); vi.mock('../../../src/services/mapsService', () => ({ buildUserAgent: () => 'TREK-Test-UA' })); const fetchMock = vi.fn(); diff --git a/server/tests/unit/services/webauthnConfig.test.ts b/server/tests/unit/services/webauthnConfig.test.ts index 157645a42..6e2f0f904 100644 --- a/server/tests/unit/services/webauthnConfig.test.ts +++ b/server/tests/unit/services/webauthnConfig.test.ts @@ -24,9 +24,10 @@ vi.mock('../../../src/db/database', () => ({ }, })); -vi.mock('../../../src/services/notifications', () => ({ - getAppUrl: () => appUrlRef.value, -})); +vi.mock('../../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAppUrl: () => appUrlRef.value }; +}); import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { resolveWebauthnConfig, isPasskeyConfigured } from '../../../src/services/webauthnConfig';