diff --git a/server/CLAUDE.md b/server/CLAUDE.md index dc948a7f6..9ffe35e45 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -84,7 +84,7 @@ SQLite is the engine today and Postgres is the planned second one. Code under `s **Never read `process.env` in `src/**`** — ESLint errors on it. All env access goes through `src/app-config/`: -- Every variable has exactly one owner. Nest classes inject a boot-stable namespace token from `src/nest/app-config/` (`httpConfig`, `storageConfig`) for values frozen per app build, or use `RuntimeEnvService` / `readEnv()` for everything else (DEMO_MODE, NODE_ENV, OIDC_*, ...). A token-owned variable is derived in `boot-derive.ts` and never by `readEnv()`; `tests/unit/app-config/config-ownership.test.ts` fails on a variable read through both, and on a token nothing injects. +- Every variable has exactly one owner. Nest classes inject a boot-stable namespace token from `src/nest/app-config/` (`httpConfig`, `storageConfig`, `transitConfig`, `kitineraryConfig`) for values frozen per app build, or use `RuntimeEnvService` / `readEnv()` for everything else (DEMO_MODE, NODE_ENV, OIDC_*, ...). A token-owned variable is derived in `boot-derive.ts` and never by `readEnv()`; `tests/unit/app-config/config-ownership.test.ts` fails on a variable read through both, and on a token nothing injects. - `server/.env.example` documents every variable `env.schema.ts` validates; `env-reference.test.ts` fails on drift either way. - Everything else calls `readEnv()` — live, uncached, per call. - Validation is fail-fast at boot only (`boot-validate.ts`, imported by `index.ts` right after dotenv): malformed values abort startup; unset/blank defaults. Never wire validation into `buildApp()` or `ConfigModule.forRoot`, and never `cache`/snapshot a runtime-toggled value (it breaks the env-mutating tests). diff --git a/server/src/app-config/README.md b/server/src/app-config/README.md index a7834c034..3147d4eb0 100644 --- a/server/src/app-config/README.md +++ b/server/src/app-config/README.md @@ -101,12 +101,31 @@ Every variable has exactly one owner, and the owner decides how it is read: `boot-derive.ts`): boot-stable values that only Nest classes need. The token is snapshotted per built app and injected (`@Inject(storageConfig.KEY)`, or `app.get(httpConfig.KEY)` for the pre-init Express layer). Today: - `httpConfig` (TRUST_PROXY, HSTS_INCLUDE_SUBDOMAINS, HTTP_KEEP_ALIVE_TIMEOUT_MS) and `storageConfig` - (TREK_PLACE_PHOTO_DIR). + `httpConfig` (TRUST_PROXY, HSTS_INCLUDE_SUBDOMAINS, HTTP_KEEP_ALIVE_TIMEOUT_MS), + `storageConfig` (TREK_PLACE_PHOTO_DIR), `transitConfig` (TRANSIT_API_URL, + for TransitService) and `kitineraryConfig` (KITINERARY_EXTRACTOR_PATH and + PATH, for KitineraryExtractorService's binary probe). A module whose provider + injects one imports `AppConfigModule`, so it also builds on its own in a test. - **`readEnv()` / `RuntimeEnvService`** (`derive.ts`): everything else, both - the runtime-toggled values and the boot-stable ones that code outside the - container needs (config.ts, the database, src/mcp, the SSRF guard), which - freeze them in module-top consts. + the runtime-toggled values and the boot-stable ones that code outside a Nest + provider reads, which freezes them in module-top consts. Each of those has a + reader the container cannot inject into: + - PORT, HOST: `index.ts`, before the app exists. + - SESSION_DURATION(_REMEMBER), DEFAULT_LANGUAGE, ENCRYPTION_KEY: `src/config.ts`, + a plain module evaluated at process start. + - MCP_*: `src/mcp/`, the process-wide session state. + - TREK_PLUGIN_RPC_*/LOG_*/MAX_RSS_MB: the plugin host and supervisor, which + are deliberately not Nest (the sandbox boundary). + - TREK_PLUGIN_REGISTRY_URL: `PluginRegistryService` is a provider, but several + plugin suites build it by hand with a trailing `@Optional()` UnitOfWork, so + a required token parameter cannot go in without reordering that signature. + - TREK_WIKI_DIR: `nest/help/wiki.ts`, a plain module the help MCP tools call. + - BACKUP_*: `nest/backup/backup-archive.ts`, which the first-start restore + (`boot-restore.ts`, called from `index.ts`) runs before the database opens. + - LOG_LEVEL: the logger, imported everywhere. + - ALLOW_INTERNAL_NETWORK, ALLOW_LINK_LOCAL_IPS: the SSRF guard in `utils/`. + - TREK_DB_FILE, TREK_DB_JOURNAL_MODE, TREK_DB_SYNCHRONOUS: the database, + opened before the container. `deriveAll()` never reads a variable a token owns, and a token is dropped once nothing injects it. `tests/unit/app-config/config-ownership.test.ts` records @@ -129,7 +148,7 @@ variable is token-owned, module-top `readEnv()` consts elsewhere): PORT, HOST, TRUST_PROXY, HSTS_INCLUDE_SUBDOMAINS, HTTP_KEEP_ALIVE_TIMEOUT_MS, SESSION_DURATION(_REMEMBER), MCP_SESSION_TTL, MCP_MAX_SESSION_PER_USER, MCP_SSE_KEEPALIVE, TREK_PLUGIN_RPC_*/LOG_*/MAX_RSS_MB, TREK_PLUGIN_REGISTRY_URL, TREK_WIKI_DIR*, TREK_PLACE_PHOTO_DIR, BACKUP_*, -TRANSIT_API_URL, LOG_LEVEL*, ALLOW_INTERNAL_NETWORK*, ALLOW_LINK_LOCAL_IPS*, DEFAULT_LANGUAGE, +TRANSIT_API_URL, KITINERARY_EXTRACTOR_PATH, LOG_LEVEL*, ALLOW_INTERNAL_NETWORK*, ALLOW_LINK_LOCAL_IPS*, DEFAULT_LANGUAGE, TREK_DB_FILE, TREK_DB_JOURNAL_MODE, TREK_DB_SYNCHRONOUS, ENCRYPTION_KEY**. (* frozen today because the consuming module captures it at import; tests that override these set them at file top, before the SUT import.) diff --git a/server/src/app-config/boot-derive.ts b/server/src/app-config/boot-derive.ts index e8e4c6c23..bc1dfa26d 100644 --- a/server/src/app-config/boot-derive.ts +++ b/server/src/app-config/boot-derive.ts @@ -9,7 +9,7 @@ * mutate mid-lifetime, belongs in derive.ts instead. */ import type { RawEnv } from './derive'; -import { parseBool, positiveIntOr } from './parsers'; +import { parseBool, positiveIntOr, stripTrailingSlashes } from './parsers'; /** * Longer than the upstream idle timeout of the common reverse proxies and load @@ -42,10 +42,32 @@ export function deriveStorage(raw: RawEnv) { }; } +/** The Transitous/MOTIS instance TransitService proxies to. */ +export function deriveTransit(raw: RawEnv) { + return { + apiBase: stripTrailingSlashes(raw.TRANSIT_API_URL || 'https://api.transitous.org'), + }; +} + +/** Where KitineraryExtractorService looks for its binary when the module starts. */ +export function deriveKitinerary(raw: RawEnv) { + return { + extractorPath: raw.KITINERARY_EXTRACTOR_PATH, + // Windows spells it Path; every other platform PATH. Split here so the + // probe gets a list and never re-implements the delimiter. + searchPath: (raw.PATH || raw.Path || '') + .split(process.platform === 'win32' ? ';' : ':') + .map((p) => p.trim()) + .filter(Boolean), + }; +} + /** One entry per token: the namespace it registers under and the function it derives with. */ export const BOOT_DERIVERS = { http: deriveHttpBoot, storage: deriveStorage, + transit: deriveTransit, + kitinerary: deriveKitinerary, } as const; export type BootNamespace = keyof typeof BOOT_DERIVERS; diff --git a/server/src/app-config/derive.ts b/server/src/app-config/derive.ts index 6a9b5490f..200bb0614 100644 --- a/server/src/app-config/derive.ts +++ b/server/src/app-config/derive.ts @@ -279,7 +279,6 @@ export function deriveWebauthn(raw: RawEnv) { export function deriveIntegrations(raw: RawEnv) { return { unsplashAccessKey: raw.UNSPLASH_ACCESS_KEY?.trim(), - transitApiBase: stripTrailingSlashes(raw.TRANSIT_API_URL || 'https://api.transitous.org'), // Trimmed before the default fires: the schema validates the trimmed value and // treats a blank one as unset, so a padded or whitespace-only value would // otherwise pass startup and then be the string that cannot be fetched. @@ -288,7 +287,6 @@ export function deriveIntegrations(raw: RawEnv) { // Longer than the `[timeout:20]` the query itself carries, or we abort an answer the // mirror was still allowed to be working on. See OVERPASS_QUERY_TIMEOUT_S. overpassTimeoutMs: positiveNumberOr(raw.OVERPASS_TIMEOUT_MS, 25000), - kitineraryExtractorPath: raw.KITINERARY_EXTRACTOR_PATH, /** * One ceiling for a model call, replacing the three per-client constants * that used to disagree. The default is deliberately generous: heavier @@ -297,12 +295,9 @@ export function deriveIntegrations(raw: RawEnv) { * rejects a fractional value. */ llmTimeoutMs: Math.floor(positiveNumberOr(raw.LLM_TIMEOUT_MS, 900_000)), - // Windows spells it Path; every other platform PATH. Split here so callers - // get a list and never re-implement the delimiter. - searchPath: (raw.PATH || raw.Path || '') - .split(process.platform === 'win32' ? ';' : ':') - .map(p => p.trim()) - .filter(Boolean), + // TRANSIT_API_URL, KITINERARY_EXTRACTOR_PATH and PATH are boot-stable and + // read only by Nest providers, so the transitConfig and kitineraryConfig + // tokens own them (boot-derive.ts). }; } diff --git a/server/src/nest/app-config/index.ts b/server/src/nest/app-config/index.ts index 7575353eb..4daf3a97e 100644 --- a/server/src/nest/app-config/index.ts +++ b/server/src/nest/app-config/index.ts @@ -1,4 +1,4 @@ export { AppConfigModule } from './app-config.module'; export { RuntimeEnvService } from './runtime-env.service'; export { DataPathsService } from './data-paths.service'; -export { httpConfig, storageConfig, BOOT_STABLE_TOKENS } from './tokens'; +export { httpConfig, storageConfig, transitConfig, kitineraryConfig, BOOT_STABLE_TOKENS } from './tokens'; diff --git a/server/src/nest/app-config/tokens.ts b/server/src/nest/app-config/tokens.ts index 63be767f4..935301d85 100644 --- a/server/src/nest/app-config/tokens.ts +++ b/server/src/nest/app-config/tokens.ts @@ -27,5 +27,9 @@ import { BOOT_DERIVERS } from '../../app-config/boot-derive'; export const httpConfig = registerAs('http', () => BOOT_DERIVERS.http(process.env)); /** The storage registry's conditional place-photo backend (TREK_PLACE_PHOTO_DIR). */ export const storageConfig = registerAs('storage', () => BOOT_DERIVERS.storage(process.env)); +/** TransitService's upstream (TRANSIT_API_URL). */ +export const transitConfig = registerAs('transit', () => BOOT_DERIVERS.transit(process.env)); +/** KitineraryExtractorService's binary probe (KITINERARY_EXTRACTOR_PATH, then PATH). */ +export const kitineraryConfig = registerAs('kitinerary', () => BOOT_DERIVERS.kitinerary(process.env)); -export const BOOT_STABLE_TOKENS = [httpConfig, storageConfig]; +export const BOOT_STABLE_TOKENS = [httpConfig, storageConfig, transitConfig, kitineraryConfig]; diff --git a/server/src/nest/booking-import/kitinerary-extractor.module.ts b/server/src/nest/booking-import/kitinerary-extractor.module.ts index 8cb88c244..46470cdf4 100644 --- a/server/src/nest/booking-import/kitinerary-extractor.module.ts +++ b/server/src/nest/booking-import/kitinerary-extractor.module.ts @@ -1,11 +1,14 @@ import { Module } from '@nestjs/common'; +import { AppConfigModule } from '../app-config/app-config.module'; import { KitineraryExtractorService } from './kitinerary-extractor.service'; -/** The kitinerary binary probe on its own. It has no dependencies, and the - * feature-flag endpoint in health/ needs nothing else from booking-import — - * importing the whole domain there would drag llm-parse, reservations, budget, - * maps and places along for one `isAvailable()`. */ +/** The kitinerary binary probe on its own. Its only dependency is the + * kitineraryConfig token (AppConfigModule), and the feature-flag endpoint in + * health/ needs nothing else from booking-import — importing the whole domain + * there would drag llm-parse, reservations, budget, maps and places along for + * one `isAvailable()`. */ @Module({ + imports: [AppConfigModule], providers: [KitineraryExtractorService], exports: [KitineraryExtractorService], }) diff --git a/server/src/nest/booking-import/kitinerary-extractor.service.ts b/server/src/nest/booking-import/kitinerary-extractor.service.ts index 423f8e72c..a9a7c7067 100644 --- a/server/src/nest/booking-import/kitinerary-extractor.service.ts +++ b/server/src/nest/booking-import/kitinerary-extractor.service.ts @@ -1,10 +1,11 @@ -import { Injectable, OnModuleInit } from '@nestjs/common'; +import { Inject, Injectable, OnModuleInit } from '@nestjs/common'; +import type { ConfigType } from '@nestjs/config'; import { execFile } from 'node:child_process'; import { existsSync, readdirSync, writeFileSync, unlinkSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, extname } from 'node:path'; import { randomUUID } from 'node:crypto'; -import { readEnv } from '../../app-config'; +import { kitineraryConfig } from '../app-config/tokens'; import { logDebug } from '../audit/audit-log.logger'; import { execFileSync } from 'node:child_process'; import { promisify } from 'node:util'; @@ -29,8 +30,11 @@ export class KitineraryExtractorService implements OnModuleInit { * one failure that otherwise looks exactly like "not installed". */ private configuredPath: string | null = null; + /** KITINERARY_EXTRACTOR_PATH and PATH, frozen per built app like the probe result itself. */ + constructor(@Inject(kitineraryConfig.KEY) private readonly kitineraryEnv: ConfigType) {} + onModuleInit() { - this.configuredPath = readEnv().integrations.kitineraryExtractorPath || null; + this.configuredPath = this.kitineraryEnv.extractorPath || null; this.binaryPath = this.findBinary(); if (this.binaryPath) { this.binaryVersion = this.probeVersion(this.binaryPath); @@ -131,7 +135,7 @@ export class KitineraryExtractorService implements OnModuleInit { } private findBinary(): string | null { - const envPath = readEnv().integrations.kitineraryExtractorPath; + const envPath = this.kitineraryEnv.extractorPath; if (envPath) { if (existsSync(envPath)) return envPath; console.warn(`[KItinerary] KITINERARY_EXTRACTOR_PATH="${envPath}" not found`); @@ -153,7 +157,7 @@ export class KitineraryExtractorService implements OnModuleInit { // this branch is dead there) anyone who could write to a PATH directory could // have their binary run as the TREK user. Probing the concrete file with // execFileSync also drops the /bin/sh hop the old execSync string needed. - for (const dir of readEnv().integrations.searchPath) { + for (const dir of this.kitineraryEnv.searchPath) { const candidate = join(dir, BINARY_NAME); if (!existsSync(candidate)) continue; try { diff --git a/server/src/nest/transit/transit.module.ts b/server/src/nest/transit/transit.module.ts index 23ba3c4f3..a3ef895e6 100644 --- a/server/src/nest/transit/transit.module.ts +++ b/server/src/nest/transit/transit.module.ts @@ -13,12 +13,15 @@ import { AppSettings } from '../../db/entities/AppSettings.entity'; import { Users } from '../../db/entities/Users.entity'; import { Trips } from '../../db/entities/Trips.entity'; import { GoogleQuotaModule } from '../google-quota/google-quota.module'; +import { AppConfigModule } from '../app-config/app-config.module'; /** * Transit domain (#1065) — the Transitous/MOTIS proxy, with the optional * Google backend (#1699) behind it. TransitMcp carries the * decorator-registered MCP tools; DaysModule/ReservationsModule feed * create_transit_journey. Exports TransitService for in-container consumers. + * AppConfigModule provides the transitConfig token (TRANSIT_API_URL) that + * TransitService injects. * * MikroOrmModule.forFeature([AppSettings, Users, Trips]): GoogleTransitProvider * passes its own AppSettingsRepository/UsersRepository to @@ -30,7 +33,7 @@ import { GoogleQuotaModule } from '../google-quota/google-quota.module'; */ @Module({ // DaysModule + ReservationsModule: TransitMcp's create_transit_journey injects both. - imports: [McpSharedModule, RateLimitModule, DaysModule, ReservationsModule, AuthModule, MikroOrmModule.forFeature([AppSettings, Users, Trips]), GoogleQuotaModule], + imports: [AppConfigModule, McpSharedModule, RateLimitModule, DaysModule, ReservationsModule, AuthModule, MikroOrmModule.forFeature([AppSettings, Users, Trips]), GoogleQuotaModule], controllers: [TransitController], providers: [TransitService, TransitMcp, GoogleTransitProvider], exports: [TransitService], diff --git a/server/src/nest/transit/transit.service.ts b/server/src/nest/transit/transit.service.ts index 451eb4f39..c618ca252 100644 --- a/server/src/nest/transit/transit.service.ts +++ b/server/src/nest/transit/transit.service.ts @@ -1,5 +1,7 @@ -import { Injectable } from '@nestjs/common'; -import { getAppUrl, readEnv } from '../../app-config'; +import { Inject, Injectable } from '@nestjs/common'; +import type { ConfigType } from '@nestjs/config'; +import { getAppUrl } from '../../app-config'; +import { transitConfig } from '../app-config/tokens'; import { buildUserAgent } from '../maps/maps.helpers'; import { GoogleTransitProvider } from './google-transit.provider'; import type { TransitProvider } from '@trek/shared'; @@ -37,15 +39,13 @@ import { * contact info, which we send once from the server), and responses are mapped * to a compact shape so the client isn't coupled to the MOTIS schema. * - * Folded 1:1 from the legacy services/transitService.ts. The base URL, the - * lazy User-Agent memo and the response cache stay module-scoped on purpose - * (the permissions/exchange-rates precedent): the DI singleton and any future - * non-Nest caller share one cache, and the base URL keeps its frozen-at-import - * legacy timing. + * Folded 1:1 from the legacy services/transitService.ts. The lazy User-Agent + * memo and the response cache stay module-scoped on purpose (the + * permissions/exchange-rates precedent): the DI singleton and any future + * non-Nest caller share one cache. The base URL (TRANSIT_API_URL) is + * boot-stable and comes from the transitConfig token, frozen per built app. */ -// Frozen at import on purpose (legacy timing). -const TRANSIT_API_BASE = readEnv().integrations.transitApiBase; let userAgent: string | null = null; function getUserAgent(): string { @@ -95,8 +95,8 @@ function isCoord(v: string): boolean { // megabyte; anything bigger is a misbehaving provider, not data we want to map. const MAX_RESPONSE_BYTES = 5_000_000; -async function upstream(path: string, params: URLSearchParams): Promise { - const url = `${TRANSIT_API_BASE}${path}?${params}`; +async function upstream(base: string, path: string, params: URLSearchParams): Promise { + const url = `${base}${path}?${params}`; const res = await fetch(url, { headers: { 'User-Agent': getUserAgent(), Accept: 'application/json' }, signal: AbortSignal.timeout(8000), @@ -154,7 +154,10 @@ function mapStop(p: MotisPlaceRaw | undefined, kind: 'departure' | 'arrival'): T @Injectable() export class TransitService { - constructor(private readonly google: GoogleTransitProvider) {} + constructor( + private readonly google: GoogleTransitProvider, + @Inject(transitConfig.KEY) private readonly transitEnv: ConfigType, + ) {} /** * Station/place search for the from/to pickers. `near` biases results. @@ -192,7 +195,7 @@ export class TransitService { const cached = cacheGet(key); if (cached) return { ...(cached as { results: TransitPlace[] }), provider }; - const raw = (await upstream('/api/v1/geocode', params)) as Array<{ + const raw = (await upstream(this.transitEnv.apiBase, '/api/v1/geocode', params)) as Array<{ name?: string; lat?: number; lon?: number; @@ -261,7 +264,7 @@ export class TransitService { const cached = cacheGet(key); if (cached) return { ...(cached as { itineraries: TransitItinerary[] }), provider: 'transitous' }; - const raw = (await upstream('/api/v6/plan', params)) as { + const raw = (await upstream(this.transitEnv.apiBase, '/api/v6/plan', params)) as { itineraries?: Array<{ duration?: number; startTime?: string; diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index d0be121e3..71db16df8 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -51,6 +51,7 @@ import { TodoService } from '../../src/nest/todo/todo.service'; import { TransitMcp } from '../../src/nest/transit/transit.mcp'; import { GoogleTransitProvider } from '../../src/nest/transit/google-transit.provider'; import { TransitService } from '../../src/nest/transit/transit.service'; +import { transitConfig } from '../../src/nest/app-config/tokens'; import { FilesService } from '../../src/nest/files/files.service'; import { FilesMcp } from '../../src/nest/files/files.mcp'; import { TripsMcp } from '../../src/nest/trips/trips.mcp'; @@ -491,7 +492,7 @@ export async function createMcpTestRegistry(): Promise { // (`findUsernameEmail`, UM11's precedent) added in its place. usersRepo, authService, addonsService, ), - new TransitMcp(new TransitService(new GoogleTransitProvider(appSettings, usersRepo, noGoogleQuota)), daysService, reservationsService, await createTestTripsRepo(db), authService, guards), + new TransitMcp(new TransitService(new GoogleTransitProvider(appSettings, usersRepo, noGoogleQuota), transitConfig()), daysService, reservationsService, await createTestTripsRepo(db), authService, guards), new AtlasMcp(new AtlasService( await createTestBucketListRepo(db), await createTestHiddenCountriesRepo(db), await createTestHiddenRegionsRepo(db), await createTestVisitedCountriesRepo(db), diff --git a/server/tests/unit/app-config/derive.test.ts b/server/tests/unit/app-config/derive.test.ts index 53189ad9c..2b6ffb572 100644 --- a/server/tests/unit/app-config/derive.test.ts +++ b/server/tests/unit/app-config/derive.test.ts @@ -21,7 +21,7 @@ import { derivePush, deriveAll, } from '../../../src/app-config/derive'; -import { deriveHttpBoot, deriveStorage } from '../../../src/app-config/boot-derive'; +import { deriveHttpBoot, deriveKitinerary, deriveStorage, deriveTransit } from '../../../src/app-config/boot-derive'; // These tests PIN the exact legacy coercions each derived field replaced. // If one fails after an edit, the edit changed runtime behavior — fix the @@ -229,10 +229,8 @@ describe('derivePlugins', () => { }); describe('deriveIntegrations', () => { - it('pins unsplash trim, transit base strip + default, nominatim trim + default, overpass timeout', () => { + it('pins unsplash trim, nominatim trim + default, overpass timeout', () => { expect(deriveIntegrations({ UNSPLASH_ACCESS_KEY: ' key ' }).unsplashAccessKey).toBe('key'); - expect(deriveIntegrations({}).transitApiBase).toBe('https://api.transitous.org'); - expect(deriveIntegrations({ TRANSIT_API_URL: 'https://t.example//' }).transitApiBase).toBe('https://t.example'); expect(deriveIntegrations({}).nominatimUrl).toBe('https://nominatim.openstreetmap.org'); // Padded and whitespace-only both come back from a compose file or a ConfigMap, // and the schema already called the second one unset. @@ -253,6 +251,28 @@ describe('deriveIntegrations', () => { }); }); +describe('deriveTransit (transitConfig token)', () => { + it('TRANSIT_API_URL: trailing slashes stripped, Transitous unless set', () => { + expect(deriveTransit({}).apiBase).toBe('https://api.transitous.org'); + expect(deriveTransit({ TRANSIT_API_URL: 'https://t.example//' }).apiBase).toBe('https://t.example'); + }); +}); + +describe('deriveKitinerary (kitineraryConfig token)', () => { + const sep = process.platform === 'win32' ? ';' : ':'; + + it('KITINERARY_EXTRACTOR_PATH passes through as given, unset stays unset', () => { + expect(deriveKitinerary({}).extractorPath).toBeUndefined(); + expect(deriveKitinerary({ KITINERARY_EXTRACTOR_PATH: '/opt/ki' }).extractorPath).toBe('/opt/ki'); + }); + + it('splits PATH (or the Windows Path) on the platform delimiter and drops blank entries', () => { + expect(deriveKitinerary({ PATH: ['/a', ' /b ', '', '/c'].join(sep) }).searchPath).toEqual(['/a', '/b', '/c']); + expect(deriveKitinerary({ Path: ['/w1', '/w2'].join(sep) }).searchPath).toEqual(['/w1', '/w2']); + expect(deriveKitinerary({}).searchPath).toEqual([]); + }); +}); + describe('deriveFiles (#1364)', () => { it('defaults the upload limit to 50 MB and takes FILE_UPLOAD_LIMIT_MB', () => { expect(deriveFiles({}).uploadLimitMb).toBe(50); diff --git a/server/tests/unit/nest/booking-import/kitinerary-extractor.service.test.ts b/server/tests/unit/nest/booking-import/kitinerary-extractor.service.test.ts index 45e569f06..cae422e03 100644 --- a/server/tests/unit/nest/booking-import/kitinerary-extractor.service.test.ts +++ b/server/tests/unit/nest/booking-import/kitinerary-extractor.service.test.ts @@ -8,11 +8,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; * at all, and every branch that resolves it is a filesystem lookup that behaves * differently on the three platforms TREK ships to. */ -const { existsSync, readdirSync, readEnv, execFileSync, execFile, logDebug } = vi.hoisted(() => ({ +const { existsSync, readdirSync, execFileSync, execFile, logDebug } = vi.hoisted(() => ({ logDebug: vi.fn(), existsSync: vi.fn(), readdirSync: vi.fn(), - readEnv: vi.fn(), execFileSync: vi.fn(), // A plain function, because the service promisifies it at module load. execFile: vi.fn(), @@ -29,7 +28,6 @@ vi.mock('node:fs', () => ({ // green-or-red depending on whether the developer happens to have KItinerary // installed. vi.mock('node:child_process', () => ({ execFileSync, execFile })); -vi.mock('../../../../src/app-config', () => ({ readEnv })); // The logger reads readEnv().app.logLevel while its module is evaluated, so it // has to be mocked rather than imported for real. vi.mock('../../../../src/nest/audit/audit-log.logger', () => ({ @@ -44,9 +42,10 @@ import { KitineraryExtractorService } from '../../../../src/nest/booking-import/ // string would never match. const onPath = (dir: string) => join(dir, 'kitinerary-extractor'); -function boot(env: { kitineraryExtractorPath?: string; searchPath?: string[] } = {}) { - readEnv.mockReturnValue({ integrations: { searchPath: [], ...env } }); - const svc = new KitineraryExtractorService(); +// What the kitineraryConfig token hands the service: KITINERARY_EXTRACTOR_PATH +// and the split PATH, frozen when the app is built. +function boot(env: { extractorPath?: string; searchPath?: string[] } = {}) { + const svc = new KitineraryExtractorService({ extractorPath: undefined, searchPath: [], ...env }); svc.onModuleInit(); return svc; } @@ -61,14 +60,14 @@ beforeEach(() => { describe('KitineraryExtractorService binary probe', () => { it('KIT-EXT-001: takes the configured path when it exists', () => { existsSync.mockImplementation((p: string) => p === '/opt/kitinerary-extractor'); - expect(boot({ kitineraryExtractorPath: '/opt/kitinerary-extractor' }).isAvailable()).toBe(true); + expect(boot({ extractorPath: '/opt/kitinerary-extractor' }).isAvailable()).toBe(true); }); it('KIT-EXT-002: an explicitly configured path that is missing disables the feature outright', () => { // It deliberately does NOT fall through to the search: somebody who set the // variable meant that binary, and silently using another one would hide the // typo behind a working feature. - const svc = boot({ kitineraryExtractorPath: '/nope/kitinerary-extractor' }); + const svc = boot({ extractorPath: '/nope/kitinerary-extractor' }); expect(svc.isAvailable()).toBe(false); expect(readdirSync).not.toHaveBeenCalled(); }); @@ -138,7 +137,7 @@ describe('KitineraryExtractorService diagnostics', () => { existsSync.mockImplementation((p: string) => p === '/opt/ki'); execFileSync.mockReturnValue(Buffer.from('kitinerary-extractor 6.3.3\n')); - expect(boot({ kitineraryExtractorPath: '/opt/ki' }).describe()).toEqual({ + expect(boot({ extractorPath: '/opt/ki' }).describe()).toEqual({ available: true, path: '/opt/ki', version: '6.3.3', configuredPath: '/opt/ki', }); }); @@ -147,7 +146,7 @@ describe('KitineraryExtractorService diagnostics', () => { existsSync.mockImplementation((p: string) => p === '/opt/ki'); execFileSync.mockImplementation(() => { throw new Error('nope'); }); - const described = boot({ kitineraryExtractorPath: '/opt/ki' }).describe(); + const described = boot({ extractorPath: '/opt/ki' }).describe(); expect(described.available).toBe(true); expect(described.version).toBeNull(); }); @@ -155,7 +154,7 @@ describe('KitineraryExtractorService diagnostics', () => { it('KIT-EXT-012: a configured path that does not exist is reported as such', () => { existsSync.mockReturnValue(false); - expect(boot({ kitineraryExtractorPath: '/nope/ki' }).describe()).toEqual({ + expect(boot({ extractorPath: '/nope/ki' }).describe()).toEqual({ available: false, path: null, version: null, configuredPath: '/nope/ki', }); }); @@ -184,7 +183,7 @@ describe('KitineraryExtractorService stderr handling', () => { execFile.mockImplementation((_bin: string, _args: string[], _opts: unknown, cb: (e: unknown, r: unknown) => void) => { cb(null, { stdout: '[]', stderr }); }); - await boot({ kitineraryExtractorPath: '/opt/ki' }).extract(Buffer.from(''), 'booking.eml'); + await boot({ extractorPath: '/opt/ki' }).extract(Buffer.from(''), 'booking.eml'); } it('KIT-EXT-015: passes every raw line to the debug log, script errors included', async () => { diff --git a/server/tests/unit/nest/google-transit.provider.test.ts b/server/tests/unit/nest/google-transit.provider.test.ts index f04509e61..693751978 100644 --- a/server/tests/unit/nest/google-transit.provider.test.ts +++ b/server/tests/unit/nest/google-transit.provider.test.ts @@ -16,6 +16,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { GoogleTransitProvider, clearGoogleTransitCache } from '../../../src/nest/transit/google-transit.provider'; import { decodePolyline, encodePolyline } from '../../../src/nest/transit/transit.helpers'; import { TransitService } from '../../../src/nest/transit/transit.service'; +import { transitConfig } from '../../../src/nest/app-config/tokens'; import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; import { noGoogleQuota } from '../../helpers/google-quota'; @@ -193,13 +194,13 @@ describe('activation', () => { it('GTRANSIT-005: TransitService routes to Google only when it is active', async () => { fetchMock.mockResolvedValue(okJson(subwayRoute())); - const service = new TransitService(makeProvider(GOOGLE_SETTINGS)); + const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig()); await service.plan({ from: FROM, to: TO }); expect(fetchMock.mock.calls[0][0]).toBe('https://routes.googleapis.com/directions/v2:computeRoutes'); }); it('GTRANSIT-006: TransitService still validates the request before dispatching', async () => { - const service = new TransitService(makeProvider(GOOGLE_SETTINGS)); + const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig()); await expect(service.plan({ from: 'nowhere', to: TO })).rejects.toThrow('from must be "lat,lng"'); await expect(service.plan({ from: FROM, to: TO, modes: 'ROCKET' })).rejects.toThrow('unsupported transit mode'); expect(fetchMock).not.toHaveBeenCalled(); @@ -477,14 +478,14 @@ describe('geocode', () => { it('GTRANSIT-020: repeat lookups of the same station are answered from cache', async () => { fetchMock.mockResolvedValue(okJson(places)); - const service = new TransitService(makeProvider(GOOGLE_SETTINGS)); + const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig()); await service.geocode('Kyobashi', 'en', undefined, 1); await service.geocode('Kyobashi', 'en', undefined, 1); expect(fetchMock).toHaveBeenCalledTimes(1); }); it('GTRANSIT-021: a short query never reaches Google', async () => { - const service = new TransitService(makeProvider(GOOGLE_SETTINGS)); + const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig()); // The provider is still named: the answer comes from the length guard, not // from a backend that declined. expect(await service.geocode('a', 'en', undefined, 1)).toEqual({ results: [], provider: 'google' }); @@ -684,7 +685,7 @@ describe('walk coalescing', () => { describe('provider reporting', () => { it('GTRANSIT-036: names Google when Google answered', async () => { fetchMock.mockResolvedValue(okJson(subwayRoute())); - const service = new TransitService(makeProvider(GOOGLE_SETTINGS)); + const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig()); const planned = await service.plan({ from: FROM, to: TO }, 'en', 1); expect(planned.provider).toBe('google'); @@ -696,7 +697,7 @@ describe('provider reporting', () => { fetchMock.mockResolvedValue(okJson({ itineraries: [] })); // Google selected, but no key resolves — the request goes to Transitous and // says so, instead of leaving an empty result to be blamed on Google. - const service = new TransitService(makeProvider({ transit_provider: 'google' })); + const service = new TransitService(makeProvider({ transit_provider: 'google' }), transitConfig()); const planned = await service.plan({ from: '48.8583,2.3470', to: '48.8809,2.3553' }, 'en', 1); expect(planned.provider).toBe('transitous'); expect(planned.itineraries).toEqual([]); @@ -705,7 +706,7 @@ describe('provider reporting', () => { it('GTRANSIT-038: a cached answer still names its backend', async () => { fetchMock.mockResolvedValue(okJson({ itineraries: [] })); - const service = new TransitService(makeProvider({})); + const service = new TransitService(makeProvider({}), transitConfig()); const first = await service.plan({ from: '48.1,2.1', to: '48.2,2.2' }, 'en', 1); const second = await service.plan({ from: '48.1,2.1', to: '48.2,2.2' }, 'en', 1); expect(fetchMock).toHaveBeenCalledTimes(1); diff --git a/server/tests/unit/nest/transit.service.test.ts b/server/tests/unit/nest/transit.service.test.ts index 9fdef99eb..080efaa7a 100644 --- a/server/tests/unit/nest/transit.service.test.ts +++ b/server/tests/unit/nest/transit.service.test.ts @@ -12,6 +12,7 @@ import { deriveTransitStats, type TransitLeg } from '../../../src/nest/transit/transit.helpers'; import { GoogleTransitProvider } from '../../../src/nest/transit/google-transit.provider'; import { TransitService } from '../../../src/nest/transit/transit.service'; +import { transitConfig } from '../../../src/nest/app-config/tokens'; import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; @@ -29,7 +30,7 @@ const fetchMock = vi.fn(); // exercising the MOTIS path — the Google branch has its own suite. const noAppSettings = { getValue: async () => null } as unknown as AppSettingsRepository; const noUsers = { getApiKeyColumn: async () => null } as unknown as UsersRepository; -const svc = new TransitService(new GoogleTransitProvider(noAppSettings, noUsers, noGoogleQuota)); +const svc = new TransitService(new GoogleTransitProvider(noAppSettings, noUsers, noGoogleQuota), transitConfig()); beforeEach(() => { vi.stubGlobal('fetch', fetchMock); @@ -272,3 +273,20 @@ describe('quirk repairs (DI fold fix pass)', () => { expect(fetchMock.mock.calls.length).toBe(fetchesBeforeTouch + 1); }); }); + +describe('upstream base URL', () => { + it('TRANSIT-SVC-016: calls the instance the transitConfig token names (TRANSIT_API_URL)', async () => { + const own = new TransitService(new GoogleTransitProvider(noAppSettings, noUsers, noGoogleQuota), { + apiBase: 'https://motis.internal.example', + }); + fetchMock.mockResolvedValueOnce(okJson([])); + await own.geocode('own-motis-station'); + expect(String(fetchMock.mock.calls[0][0])).toMatch(/^https:\/\/motis\.internal\.example\/api\/v1\/geocode\?/); + }); + + it('TRANSIT-SVC-017: defaults to Transitous when TRANSIT_API_URL is unset', async () => { + fetchMock.mockResolvedValueOnce(okJson([])); + await svc.geocode('default-base-station'); + expect(String(fetchMock.mock.calls[0][0])).toMatch(/^https:\/\/api\.transitous\.org\/api\/v1\/geocode\?/); + }); +});