mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 14:36:31 +00:00
fix(llm-parse): one configurable ceiling for a model call, and undici honours it
The timeout for a model call was spelled three times and adjustable nowhere: 120s in the Anthropic client, 300s in the OpenAI-compatible one, 300s in the Ollama one. None of them was the ceiling that actually applied, either — every one of those calls goes through the pinned undici dispatcher, and an undici Agent stops waiting for response headers after five minutes by default, so a client constant set above that never got the chance to fire. - LLM_TIMEOUT_MS (default 900000) replaces the three constants and is passed to the dispatcher as headersTimeout/bodyTimeout, so the abort signal and undici are governed by the same setting. The default is deliberately generous: heavier parsing work should fit without a code change, and an admin who would rather fail fast can lower it. - The ceiling stays on the model lane. safeFetchAdminConfigured is also the OIDC lane (discovery, token, userinfo, JWKS) and the plugin OAuth token exchange, and that one sends no AbortSignal of its own, so it keeps undici's five minutes rather than inheriting a model setting. - The derived value is floored: the schema accepts any positive number and undici rejects a fractional headersTimeout.
This commit is contained in:
@@ -81,7 +81,8 @@ TREK_MANAGED, PLACES_API_BASE, PLACES_API_KEY, MAPBOX_ACCESS_TOKEN, CARTO_API_KE
|
||||
TREK_PLUGINS_ENABLED / _DEV_LINK / _IGNORE_TREK_RANGE / _DIR / _DATA_DIR / TREK_PLUGIN_PERMISSIONS,
|
||||
OIDC_*, SMTP_*, FORCE_HTTPS, COOKIE_SECURE, HSTS_INCLUDE_SUBDOMAINS,
|
||||
ALLOWED_ORIGINS, UNSPLASH_ACCESS_KEY, WEBAUTHN_*, TZ, ADMIN_EMAIL,
|
||||
ADMIN_PASSWORD, IDEMPOTENCY_TTL_SECONDS, MCP_RATE_LIMIT (request-path check).
|
||||
ADMIN_PASSWORD, IDEMPOTENCY_TTL_SECONDS, MCP_RATE_LIMIT (request-path check),
|
||||
LLM_TIMEOUT_MS.
|
||||
|
||||
## Exemptions — raw `process.env` stays
|
||||
|
||||
|
||||
@@ -241,6 +241,14 @@ export function deriveIntegrations(raw: RawEnv) {
|
||||
overpassUrl: raw.OVERPASS_URL,
|
||||
overpassTimeoutMs: positiveNumberOr(raw.OVERPASS_TIMEOUT_MS, 12000),
|
||||
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
|
||||
* parsing work should fit without a code change.
|
||||
* Floored to a whole number — it reaches undici's headersTimeout, which
|
||||
* 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 || '')
|
||||
|
||||
@@ -112,6 +112,7 @@ export const envSchema = z.object({
|
||||
// non-http(s) entries today — left unvalidated to keep that behavior.
|
||||
OVERPASS_URL: anyString,
|
||||
OVERPASS_TIMEOUT_MS: positiveNumber,
|
||||
LLM_TIMEOUT_MS: positiveNumber,
|
||||
KITINERARY_EXTRACTOR_PATH: anyString,
|
||||
// The OS search path. Not configuration anybody sets for TREK — it is here so
|
||||
// the kitinerary probe can resolve its binary to an absolute path itself
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
import type { LlmExtractionClient, LlmExtractionInput } from '../llm-provider.interface';
|
||||
import { safeFetchLlm } from '../../../utils/ssrfGuard';
|
||||
import { readEnv } from '../../../app-config';
|
||||
import { toReservationList } from '../lenient-json';
|
||||
|
||||
const TIMEOUT_MS = 120_000;
|
||||
const MAX_TOKENS = 8192;
|
||||
const ANTHROPIC_VERSION = '2023-06-01';
|
||||
const TOOL_NAME = 'emit_reservations';
|
||||
@@ -51,7 +51,7 @@ export class AnthropicClient implements LlmExtractionClient {
|
||||
};
|
||||
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), TIMEOUT_MS);
|
||||
const timer = setTimeout(() => controller.abort(), readEnv().integrations.llmTimeoutMs);
|
||||
let res: Response;
|
||||
try {
|
||||
// baseUrl is user-configurable — guard it against pointing at the cloud
|
||||
|
||||
@@ -2,10 +2,8 @@ import type { LlmExtractionClient, LlmExtractionInput } from '../llm-provider.in
|
||||
import { isNuExtractModel, buildNuExtractUserText, nuExtractToKiReservations } from './nuextract';
|
||||
import { parseLenientJson, toReservationList } from '../lenient-json';
|
||||
import { safeFetchLlm } from '../../../utils/ssrfGuard';
|
||||
import { readEnv } from '../../../app-config';
|
||||
|
||||
// Generous: a local CPU model (Ollama, no GPU) may cold-load several GB and then
|
||||
// take a few minutes on a longer document before the first token.
|
||||
const TIMEOUT_MS = 300_000;
|
||||
const MAX_TOKENS = 4096;
|
||||
|
||||
/**
|
||||
@@ -110,7 +108,7 @@ export class OpenAiCompatibleClient implements LlmExtractionClient {
|
||||
|
||||
private async send(url: string, body: unknown, apiKey?: string): Promise<Response> {
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), TIMEOUT_MS);
|
||||
const timer = setTimeout(() => controller.abort(), readEnv().integrations.llmTimeoutMs);
|
||||
try {
|
||||
// baseUrl is user-configurable — guard it against pointing at the cloud
|
||||
// metadata endpoint, while still allowing a local/LAN Ollama.
|
||||
|
||||
@@ -13,8 +13,7 @@
|
||||
|
||||
import { parseLenientJson } from '../lenient-json';
|
||||
import { safeFetchLlm } from '../../../utils/ssrfGuard';
|
||||
|
||||
const TIMEOUT_MS = 300_000;
|
||||
import { readEnv } from '../../../app-config';
|
||||
|
||||
export interface EnforcedExtractInput {
|
||||
/** Ollama base URL — accepts the addon's `…/v1` form; the `/v1` suffix is stripped. */
|
||||
@@ -65,7 +64,7 @@ export async function extractEnforced(input: EnforcedExtractInput): Promise<Reco
|
||||
};
|
||||
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), TIMEOUT_MS);
|
||||
const timer = setTimeout(() => controller.abort(), readEnv().integrations.llmTimeoutMs);
|
||||
let res: Response;
|
||||
try {
|
||||
// baseUrl is user-configurable — guard it against the cloud-metadata range,
|
||||
|
||||
@@ -5,7 +5,7 @@ import { encrypt_api_key, decrypt_api_key } from '../../common/crypto/apiKeyCryp
|
||||
import { applySettingDefaults, settingDefaults } from '../settings-defaults';
|
||||
import { getAppUrl } from '../../../app-config';
|
||||
import { isPrivateIp } from '../install/safe-fetch';
|
||||
import { safeFetchLlm } from '../../../utils/ssrfGuard';
|
||||
import { safeFetchAdminConfigured } from '../../../utils/ssrfGuard';
|
||||
|
||||
/**
|
||||
* Host-brokered outbound OAuth (#plugins). A plugin becomes an OAuth *client* of a
|
||||
@@ -201,7 +201,7 @@ export class PluginOAuthService {
|
||||
// pinning the connection to the resolved IP, so a token_url that is a DNS name
|
||||
// (or IPv6 literal) pointing at metadata can't reach it and can't DNS-rebind.
|
||||
// Loopback/LAN stay reachable so a self-hosted internal IdP keeps working.
|
||||
const resp = await safeFetchLlm(cfg.tokenUrl, {
|
||||
const resp = await safeFetchAdminConfigured(cfg.tokenUrl, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/x-www-form-urlencoded', accept: 'application/json' },
|
||||
body: body.toString(),
|
||||
|
||||
@@ -178,8 +178,17 @@ function isLinkLocal(ip: string): boolean {
|
||||
* safeFetchFollow) means each hop is re-resolved, re-checked against isLinkLocal,
|
||||
* and re-pinned. An http→https upgrade or a proxy redirect between LAN hosts still
|
||||
* works because the check re-runs per hop rather than locking to the first IP.
|
||||
*
|
||||
* `responseTimeoutMs` raises undici's own five-minute ceiling for callers that
|
||||
* legitimately wait longer (see safeFetchLlm). Left unset it keeps undici's
|
||||
* default, which is what every other admin-configured endpoint wants.
|
||||
*/
|
||||
export async function safeFetchAdminConfigured(url: string, init?: RequestInit, maxRedirects = 5): Promise<Response> {
|
||||
export async function safeFetchAdminConfigured(
|
||||
url: string,
|
||||
init?: RequestInit,
|
||||
maxRedirects = 5,
|
||||
responseTimeoutMs?: number,
|
||||
): Promise<Response> {
|
||||
let currentUrl = url;
|
||||
|
||||
for (let hop = 0; ; hop++) {
|
||||
@@ -203,7 +212,7 @@ export async function safeFetchAdminConfigured(url: string, init?: RequestInit,
|
||||
throw new SsrfBlockedError('Requests to link-local / cloud-metadata addresses are not allowed');
|
||||
}
|
||||
|
||||
const dispatcher = createPinnedDispatcher(resolvedIp, true);
|
||||
const dispatcher = createPinnedDispatcher(resolvedIp, true, responseTimeoutMs);
|
||||
const response = await fetch(currentUrl, { ...init, redirect: 'manual', dispatcher } as any);
|
||||
|
||||
// Only a 3xx WITH a Location header is a redirect we follow; anything else
|
||||
@@ -230,12 +239,19 @@ export async function safeFetchAdminConfigured(url: string, init?: RequestInit,
|
||||
}
|
||||
|
||||
/**
|
||||
* The original name. Kept so no LLM call site has to change, and because the
|
||||
* shape of the problem is identical: an endpoint an admin configured, which may
|
||||
* The original name, now the model lane specifically: the same guard as
|
||||
* safeFetchAdminConfigured — an endpoint an admin configured, which may
|
||||
* legitimately live on loopback or the LAN, and must still never reach
|
||||
* link-local or a cloud metadata service.
|
||||
* link-local or a cloud metadata service — with LLM_TIMEOUT_MS as the response
|
||||
* ceiling instead of undici's five minutes, so one setting governs the whole
|
||||
* call rather than being overruled by a default nobody chose.
|
||||
*
|
||||
* The ceiling is read once per call rather than per hop, so a redirect chain is
|
||||
* measured against one value even if the variable changes mid-chain.
|
||||
*/
|
||||
export const safeFetchLlm = safeFetchAdminConfigured;
|
||||
export function safeFetchLlm(url: string, init?: RequestInit, maxRedirects = 5): Promise<Response> {
|
||||
return safeFetchAdminConfigured(url, init, maxRedirects, readEnv().integrations.llmTimeoutMs);
|
||||
}
|
||||
|
||||
/**
|
||||
* Thrown by safeFetch() when the URL is blocked by the SSRF guard.
|
||||
@@ -353,8 +369,14 @@ export async function safeFetchFollow(
|
||||
* IP. This prevents DNS rebinding (TOCTOU) by ensuring the outbound connection
|
||||
* goes to the IP we checked, not a re-resolved one.
|
||||
*/
|
||||
export function createPinnedDispatcher(resolvedIp: string, rejectUnauthorized = true): Agent {
|
||||
export function createPinnedDispatcher(resolvedIp: string, rejectUnauthorized = true, responseTimeoutMs?: number): Agent {
|
||||
return new Agent({
|
||||
// undici caps the wait for response headers at 5 minutes by default, and
|
||||
// that cap is invisible from the call site: an AbortController set to
|
||||
// fifteen still dies at five.
|
||||
...(responseTimeoutMs
|
||||
? { headersTimeout: responseTimeoutMs, bodyTimeout: responseTimeoutMs }
|
||||
: {}),
|
||||
connect: {
|
||||
rejectUnauthorized,
|
||||
lookup: (_hostname: string, opts: Record<string, unknown>, callback: Function) => {
|
||||
|
||||
Reference in New Issue
Block a user