diff --git a/CLAUDE.md b/CLAUDE.md index 7d37116..20ebb78 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -345,6 +345,7 @@ Opt-in (`Llm:Enabled=false` default), OpenAI-kompatibler Endpunkt, Rate-Limit 20 - **Globaler AI-Chat / Wissens-Assistent** (`POST /api/ai/knowledge/ask`, SSE; `GET /api/ai/knowledge/capabilities`) — seitenweiter read-only Q&A in `/ai-chat`, canvas-frei. Vier admin-toggelbare Wissensquellen (Sektion `AiKnowledge`, hot-reloadbar, alle `false`-default außer Docs/Operational): **Docs** (`DocsEnabled`), **Operational** (`OperationalEnabled`, RBAC-folder-gescoped — liefert nur die Workflow-spezifische **Definition** (`get_workflow_definition`, secret-redigiert), **statische Analyse** (`analyze_workflow`) und **Cron-Voraussage** (`get_next_scheduled_fires`); reine Listen wie "welche Workflows/Läufe/Maschinen gibt es" werden über die DB-Quelle per text2sql beantwortet), **Source-Code** (`SourceCodeEnabled`, Admin/Op), **DB / text2sql** (`DbEnabled`, Admin/Op). DB-Tools (`list_db_tables`/`get_db_table`/`execute_readonly_sql`) über `ISqlKnowledgeReader`: Schema inkl. Provider/FKs ohne Secret-Spalten; zentraler Executor-Guard (64 KiB, Single-Statement, Read-only-Whitelist + Dangerous-Token/Routine-Block), geschützte Spaltenreferenzen vor Ausführung abgelehnt, Result-Masking + `IAuditDetailsRedactor`, Row-Cap 200, valides Truncation-JSON. DB-Tools Strict mit Best-Effort-Fallback; Audit nur Query-Anzahl/Fingerprint. Sources sind nur sichtbar, wenn das aktive Profil `EnableToolCalling` gesetzt hat. - **`llmQuery`-Activity:** Engine-lokal, Prompt→Text; per-Node-Overrides `baseUrl`/`model`/`apiKey`/`maxTokens`/`temperature`/`timeoutSeconds`/`jsonMode`, **gated durch `Llm:Enabled`** (zentraler Kill-Switch). Teilt Transport + SSRF-Guard via `ILlmClientFactory`; einziger BaseUrl-Validierungspunkt ist `LlmEndpointGuard`. - **Zwei Wire-Dialekte, kein Config-Key:** `LlmEndpointGuard.ResolveEndpoint` leitet aus dem `BaseUrl`-Pfad ab, wohin gepostet wird und wer antwortet — `…/responses` → `OpenAiResponsesLlmClient` (OpenAI Responses API), sonst `OpenAiCompatibleLlmClient`; endet der Pfad schon auf `/chat/completions`, wird **nichts** mehr angehängt. Gemeinsames HTTP-Plumbing in `LlmHttpTransport`. Die vier Quirk-Fallbacks (`max_tokens`→`max_completion_tokens`, `stream_options`, `response_format`, `strict`) sind Chat-Completions-only und im Responses-Client bewusst nicht vorhanden; dieser sendet immer `store: false`. +- **Erreichbarkeit ≠ Antwortzeit:** `TimeoutSeconds` ist reines **Antwort**-Budget. Der Verbindungsaufbau hat eigene Konstanten in `LlmConnectGuard` — `ConnectPhaseTimeout` (15 s, DNS+TCP im ConnectCallback) und `HandshakeTimeout` (30 s, als `SocketsHttpHandler.ConnectTimeout`, die einzige Stelle die den TLS-Handshake binden kann). Die Ordnung `HandshakeTimeout > ConnectPhaseTimeout` ist tragend (per Test gepinnt): nur deshalb darf ein gefeuertes `ConnectTimeout` als TLS-Stufe gelesen werden. Fehler nennen die Stufe (`LLM endpoint DNS:|TCP:|TLS:`); Debug-Logging der aufgelösten Adressen unter `NodePilot.Ai.LlmConnect`. Details: `docs/claude-reference.md`. - **Hardening:** SSRF-Block (Cloud-Metadata), Proxy nur nach Opt-in (`Llm:Proxy:Mode`, default `Off`), Klartext-ApiKey-/Proxy-Passwort-Warning, Prompt-Injection-Mitigation (Schema-only, User-reviewed Insert). Drift-Schutz: `PromptCatalogDriftTest.cs`. Audit: `AI_*`-Codes. ## Workflow Import/Export diff --git a/docs/ai-features.md b/docs/ai-features.md index 56603c3..7a61be9 100644 --- a/docs/ai-features.md +++ b/docs/ai-features.md @@ -102,7 +102,7 @@ Neu-Eintippen. | `ApiKey` | `null` | OpenAI-Cloud verlangt einen Key; lokale Endpoints meist nicht. **Empfohlener Weg: Env-Var `Llm__Profiles____ApiKey`** — Klartext in der Settings-Datei löst eine Startup-Hardening-Warnung aus. | | `Model` | `gpt-4o-mini` | Wird für Script-, Workflow-Generierung und beide Chats verwendet. | | `MaxTokens` | `4096` | Cap der LLM-Antwort. Reicht für ein typisches Script und einen mittelgroßen Workflow. Bei großen Modellen (32k+ Context) gerne erhöhen. | -| `TimeoutSeconds` | `90` | HTTP-Timeout. Großzügig für lokale Modelle, klein genug um nicht ewig zu hängen. | +| `TimeoutSeconds` | `90` | **Antwort**-Budget: wie lange das Modell denken darf. Deckt ausdrücklich **nicht** den Verbindungsaufbau — der hat seine eigenen, kurzen Fristen (siehe unten). Großzügig setzen ist deshalb gefahrlos: ein unerreichbarer Endpunkt scheitert trotzdem in Sekunden. | | `EnableToolCalling` | `false` | Opt-in. Lässt die Chat-Assistenten read-only Analyse-Tools per OpenAI-Function-Calling callen (`tool_choice: auto`). Braucht ein Modell, das Function-Calling zuverlässig kann — viele kleine lokale Modelle nicht. **Pro Profil**, weil das eine Eigenschaft des Modells ist, nicht der Installation: beim Umschalten auf ein kleines lokales Modell wandert die Fähigkeit mit. | | `ToolCallMaxDepth` | `6` | Max LLM-Runden mit Tool-Calls pro Chat-Turn (Loop-Guard, gültig 1–10). Lässt bei text2sql nach Schema-Discovery noch Raum für SQL-Korrekturen. In der letzten erlaubten Runde sendet der Server **keine** `tools` → erzwingt eine Text-Antwort. | @@ -136,6 +136,39 @@ genau deshalb bleibt die Sektion hot-reloadable, wo `RestApi` (Proxy fest im Han Eine Ausnahme bleibt `Mode: System`: Änderungen an den **Windows-Proxy-Einstellungen** selbst greifen erst nach einem Dienst-Neustart, weil .NET die Systemkonfiguration prozessweit cacht. +### „Endpunkt nicht erreichbar" — welche Stufe gescheitert ist + +Den Endpunkt zu *erreichen* hat eigene Fristen, unabhängig von `TimeoutSeconds`. Das ist kein +Detail, sondern der Grund, warum die Fehlermeldung überhaupt etwas aussagt: vorher deckte ein +einziges Budget DNS, TCP, TLS **und** die Modellantwort ab, und alle vier endeten in derselben +Zeile „did not respond within {TimeoutSeconds}s". Bei einem Profil mit 360 s hieß das sechs +Minuten Stille und danach ein Satz, der auf das Modell zeigte, während in Wahrheit eine Firewall +oder ein nicht vertrautes Zertifikat im Weg stand. + +| Stufe | Frist | Meldung beginnt mit | Typische Ursache | +|---|---|---|---| +| Namensauflösung | 15 s | `LLM endpoint DNS:` | Name falsch, falsches DNS-Suffix, Resolver antwortet nicht | +| TCP-Verbindung | 15 s | `LLM endpoint TCP:` | **verworfen** = Firewall/Netzsegment; **abgelehnt** = Host da, Port/Dienst falsch | +| TLS-Handshake | 30 s | `LLM endpoint TLS:` | Client-Zertifikat verlangt, SNI-Mismatch, Middlebox, die annimmt und nicht verhandelt | +| Zertifikatsprüfung | — | `LLM endpoint TLS:` … `certificate` | Interne CA fehlt im **Maschinen**-Store des NodePilot-Hosts | +| Modellantwort | `TimeoutSeconds` | `accepted the request but sent no answer` | Modell rechnet noch — hier ist Hochsetzen die richtige Antwort | + +Die Fristen der ersten drei Stufen sind Konstanten, keine Konfigurationsschlüssel: 15 s liegen weit +jenseits jeder gesunden Auflösung oder eines TCP-Handshakes (Windows gibt bei unbeantwortetem SYN +nach ~21 s von selbst auf). Ein Wert, den man hochsetzen müsste, bedeutet ein kaputtes Netz — und +genau das sagt die Meldung jetzt. + +**Zertifikate:** NodePilot prüft gegen den Trust-Store der **Maschine**, nicht den des angemeldeten +Benutzers. Ein Zertifikat, das im Browser auf dem Arbeitsplatz akzeptiert wird, ist damit für den +Dienst noch lange nicht vertrauenswürdig — die ausstellende CA muss unter +`LocalMachine\Root` des NodePilot-Hosts liegen. + +**Mehr Details im Log:** Die aufgelösten Adressen und die Dauer je Stufe stehen auf `Debug` unter +der Kategorie `NodePilot.Ai.LlmConnect` — gezielt einschaltbar über +`Serilog:MinimumLevel:Override`, ohne den ganzen KI-Stack gesprächig zu machen. Das ist die Zeile, +die den Fall „funktioniert von meinem Rechner, nicht vom Dienst" auflöst: ein veralteter +AAAA-Eintrag und ein anderes DNS-Suffix sehen von außen identisch aus. + ### Wire-Dialekt (aus der `BaseUrl` abgeleitet) OpenAI betreibt zwei Request-Formate nebeneinander: das klassische **Chat Completions** diff --git a/docs/claude-reference.md b/docs/claude-reference.md index cc416fa..19a4a00 100644 --- a/docs/claude-reference.md +++ b/docs/claude-reference.md @@ -365,6 +365,7 @@ Background-Service-, Konfigurations- und Observability-Vertrag stehen vollständ **Hardening**: - SSRF-Block für Cloud-Metadata-IPs in **jeder** `Llm:Profiles::BaseUrl` (nicht nur der aktiven — Profilwechsel ist ein restart-freier Save). Eine geteilte Regel für Boot *und* Save-Simulation: [LlmProfileValidation.cs](src/NodePilot.Ai/LlmProfileValidation.cs), aufgerufen von `AddNodePilotAi` und `LlmConfigBootValidator`. Einziger BaseUrl-Validierungspunkt bleibt [LlmEndpointGuard.cs](src/NodePilot.Ai/LlmEndpointGuard.cs) (`NormalizeAndValidateBaseUrl`/`IsCloudMetadataEndpoint`), plus Connect-Zeit-Guard `LlmConnectGuard` in [LlmServiceCollectionExtensions.cs](src/NodePilot.Ai/LlmServiceCollectionExtensions.cs). `Enabled=true` ohne auflösbares Profil ist bewusst nur eine **Warning** — KI ist opt-in und darf den Boot nicht blockieren. - Eigener `SocketsHttpHandler` (NICHT der `RestApiHttpClientProvider` — der hat SSRF-Guards die `127.0.0.1:11434` blocken würden). Proxy-Verhalten kommt aus `Llm:Proxy:*` über `LlmConfiguredProxy`, default `Off` = Direktverbindung. +- **Erreichbarkeit ist von der Antwortzeit getrennt.** `TimeoutSeconds` ist reines Antwort-Budget; der Verbindungsaufbau hat eigene Konstanten in `LlmConnectGuard`: `ConnectPhaseTimeout` (15 s, deckt DNS + TCP im ConnectCallback) und `HandshakeTimeout` (30 s, als `SocketsHttpHandler.ConnectTimeout` — die einzige Stelle, die den **TLS-Handshake** binden kann, weil der Callback nur den rohen Transport-Stream zurückgibt). Die Ordnung `HandshakeTimeout > ConnectPhaseTimeout` ist **tragend**: nur weil DNS und TCP immer an ihrer eigenen Frist scheitern, darf `LlmHttpTransport.DescribeUnreachable` aus einem gefeuerten `ConnectTimeout` auf die TLS-Stufe schließen. Ein Test pinnt die Ordnung. Meldungspräfixe: `LLM endpoint DNS:` / `TCP:` / `TLS:`; die Modell-Stufe sagt „accepted the request but sent no answer". Debug-Logging der aufgelösten Adressen unter der Kategorie `NodePilot.Ai.LlmConnect`. - Klartext-ApiKey je Profil löst Startup-Hardening-Warning aus, analog `Smtp:Password` ([SecurityHardeningWarnings.cs](src/NodePilot.Api/Hosting/SecurityHardeningWarnings.cs)) - `SettingsSchema.IsUnchangedSecretValue` behandelt `__unchanged__` **und** die Anzeige-Maske `"********"` als „unverändert" — vorher hätte ein Client, der die GET-Antwort zurück-PUTet, die Maske als neuen Key verschlüsselt und den echten still zerstört (gilt jetzt für alle Sektionen, auch `Smtp:Password`). diff --git a/src/NodePilot.Ai/LlmHttpTransport.cs b/src/NodePilot.Ai/LlmHttpTransport.cs index 4406288..63b2120 100644 --- a/src/NodePilot.Ai/LlmHttpTransport.cs +++ b/src/NodePilot.Ai/LlmHttpTransport.cs @@ -2,6 +2,7 @@ using System.Net.Http.Headers; using System.Net.Http.Json; using System.Runtime.CompilerServices; +using System.Security.Authentication; using System.Text.Json; using Microsoft.Extensions.Logging; @@ -81,13 +82,16 @@ public async Task SendAsync( } catch (OperationCanceledException) when (!caller.IsCancellationRequested) { + // Reaching the endpoint has its own, much shorter budget (LlmConnectGuard), so by the + // time this fires the request was on the wire and the model is simply still thinking. throw new LlmException(LlmErrorKind.Timeout, - $"LLM endpoint did not respond within {_config.TimeoutSeconds}s ({_config.Endpoint.PostUrl})."); + $"LLM endpoint accepted the request but sent no answer within {_config.TimeoutSeconds}s " + + $"({_config.Endpoint.PostUrl}). The connection itself was fine — raise the profile's " + + "timeout if the model needs longer, or pick a faster model."); } catch (HttpRequestException ex) { - throw new LlmException(LlmErrorKind.Unreachable, - $"LLM endpoint unreachable ({_config.Endpoint.PostUrl}): {ex.Message}", inner: ex); + throw new LlmException(LlmErrorKind.Unreachable, DescribeUnreachable(ex), inner: ex); } if (!resp.IsSuccessStatusCode) @@ -96,6 +100,47 @@ public async Task SendAsync( return resp; } + /// + /// Turns a transport failure into a message that names which stage failed. + /// + /// Every one of these used to arrive as the same "did not respond" sentence, which is + /// why an unreachable endpoint and a slow model were indistinguishable from the UI. The stages + /// are separable because each has its own deadline: DNS and TCP fail inside + /// LlmConnectGuard with a message that already names them, certificate validation + /// raises , and the handler's ConnectTimeout can + /// only fire after those two have passed — so it means the TLS handshake stalled. + /// + internal string DescribeUnreachable(Exception ex) + { + var url = _config.Endpoint.PostUrl; + + Exception innermost = ex; + while (innermost.InnerException is not null) innermost = innermost.InnerException; + + if (innermost is AuthenticationException auth) + { + return $"LLM endpoint TLS ({url}): the server's certificate was rejected — {auth.Message} " + + "Import the issuing CA into the machine's Trusted Root store on the NodePilot host; " + + "a certificate that a browser accepts on a workstation is not automatically trusted by the service account."; + } + + // TimeoutException here is SocketsHttpHandler.ConnectTimeout. DNS and TCP carry shorter + // deadlines of their own, so they can never be what expired. + if (innermost is TimeoutException) + { + return $"LLM endpoint TLS ({url}): the TCP connection was established but the TLS handshake did not " + + $"complete within {LlmConnectGuard.HandshakeTimeout.TotalSeconds:0}s. Typical causes are an endpoint " + + "demanding a client certificate, an SNI mismatch, or a middlebox that accepts the connection and " + + "never negotiates."; + } + + // Anything from the connect guard already names its own stage; don't wrap it in a second + // sentence that says less. + return innermost is IOException io && io.Message.StartsWith("LLM endpoint ", StringComparison.Ordinal) + ? io.Message + : $"LLM endpoint unreachable ({url}): {innermost.Message}"; + } + /// /// Reads an error response's body excerpt (through the same byte cap as the success path) and /// throws the matching . Disposes . diff --git a/src/NodePilot.Ai/LlmServiceCollectionExtensions.cs b/src/NodePilot.Ai/LlmServiceCollectionExtensions.cs index b43dc42..9aa606b 100644 --- a/src/NodePilot.Ai/LlmServiceCollectionExtensions.cs +++ b/src/NodePilot.Ai/LlmServiceCollectionExtensions.cs @@ -1,8 +1,13 @@ +using System.Diagnostics; using System.Net; using System.Net.Sockets; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Serilog; +// Both namespaces above carry an ILogger; the connect guard wants the abstraction the rest of the +// stack injects, while the boot-time messages in this file stay on Serilog's static Log. +using ILogger = Microsoft.Extensions.Logging.ILogger; namespace NodePilot.Ai; @@ -18,12 +23,64 @@ namespace NodePilot.Ai; /// internal static class LlmConnectGuard { - internal static async ValueTask ConnectAsync(SocketsHttpConnectionContext ctx, CancellationToken ct) + /// + /// Deadline for name resolution and the TCP connect together — the part of the handshake this + /// callback owns. + /// + /// Why a separate budget at all. The per-call timeout (TimeoutSeconds) is + /// an answer budget: a local model chewing on a long prompt legitimately needs minutes, + /// which is why operators set it to 300+. Reaching the endpoint is not that kind of work. With + /// one shared budget, an endpoint that never completes its handshake burned the whole thing — + /// a profile at 360 s sat there for six minutes and then reported "did not respond", the same + /// sentence a slow model produces. Failing the reachability phase early, and separately, is + /// what lets the two be told apart at all. + /// + /// A constant rather than an operator knob: 15 s is far beyond any healthy DNS lookup or + /// TCP handshake (Windows gives up on an unanswered SYN after ~21 s on its own), so there is + /// nothing to tune here — a value that needs raising means the network is broken, and the + /// error now says so. + /// + internal static readonly TimeSpan ConnectPhaseTimeout = TimeSpan.FromSeconds(15); + + /// + /// for the LLM handler. Covers everything up to + /// a usable connection, which — unlike this callback — includes the TLS handshake: the + /// callback returns the raw transport stream and the handler negotiates TLS on top of it. + /// + /// Larger than on purpose, so the two never race: DNS + /// and TCP always fail on their own, named deadline. Anything that trips this one has + /// therefore already connected at the TCP level and is stuck in the handshake — an endpoint + /// demanding a client certificate, an SNI mismatch, or a middlebox that accepts the connection + /// and never speaks TLS. relies on that ordering to name the + /// stage. + /// + internal static readonly TimeSpan HandshakeTimeout = TimeSpan.FromSeconds(30); + + /// + /// Log category for the connect diagnostics. Named rather than tied to a type so an operator + /// can raise just this one to Debug (Serilog:MinimumLevel:Override) when an endpoint is + /// unreachable, without turning on debug logging for the whole AI stack. + /// + internal const string LoggerCategory = "NodePilot.Ai.LlmConnect"; + + internal static ValueTask ConnectAsync(SocketsHttpConnectionContext ctx, CancellationToken ct) + => ConnectAsync(ctx, logger: null, ct); + + internal static async ValueTask ConnectAsync( + SocketsHttpConnectionContext ctx, + ILogger? logger, + CancellationToken ct) { var endPoint = ctx.DnsEndPoint; var host = endPoint.Host; var port = endPoint.Port; + // One deadline over both phases. Linked to the caller's token so a cancelled request still + // aborts immediately; the two are told apart below by asking which one fired. + using var phase = CancellationTokenSource.CreateLinkedTokenSource(ct); + phase.CancelAfter(ConnectPhaseTimeout); + + var stopwatch = Stopwatch.StartNew(); IPAddress[] resolved; if (IPAddress.TryParse(host, out var direct)) { @@ -31,8 +88,25 @@ internal static async ValueTask ConnectAsync(SocketsHttpConnectionContex } else { - resolved = await Dns.GetHostAddressesAsync(host, ct).ConfigureAwait(false); + try + { + resolved = await Dns.GetHostAddressesAsync(host, phase.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + throw new IOException( + $"LLM endpoint DNS: resolving '{host}' did not finish within {ConnectPhaseTimeout.TotalSeconds:0}s. " + + "The name server did not answer — this is name resolution, not the LLM."); + } + catch (SocketException ex) + { + throw new IOException( + $"LLM endpoint DNS: '{host}' could not be resolved ({ex.SocketErrorCode}). " + + "Check the name, the DNS suffix search list, and that this host uses the resolver that knows it.", + ex); + } } + var dnsElapsed = stopwatch.ElapsedMilliseconds; var allowed = new List(resolved.Length); foreach (var ip in resolved) @@ -47,12 +121,38 @@ internal static async ValueTask ConnectAsync(SocketsHttpConnectionContex $"LLM SSRF guard rejected every resolved address for host '{host}': link-local addresses " + "(169.254/16 incl. cloud-metadata, IPv6 fe80::/10) are not allowed for the LLM endpoint."); + // The resolved set is the single most useful thing to know when an endpoint "works from my + // machine" but not from the service: a stale AAAA record, or a name that resolves somewhere + // else entirely under the service account's DNS suffixes, both look identical from outside. + logger?.LogDebug( + "LLM connect: {Host}:{Port} resolved to {Addresses} in {DnsMs} ms.", + host, port, string.Join(", ", allowed), dnsElapsed); + var socket = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true }; try { - await socket.ConnectAsync(allowed.ToArray(), port, ct).ConfigureAwait(false); + await socket.ConnectAsync(allowed.ToArray(), port, phase.Token).ConfigureAwait(false); + logger?.LogDebug( + "LLM connect: TCP to {Endpoint} established in {TotalMs} ms; TLS (if any) is negotiated next.", + socket.RemoteEndPoint, stopwatch.ElapsedMilliseconds); return new NetworkStream(socket, ownsSocket: true); } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + socket.Dispose(); + throw new IOException( + $"LLM endpoint TCP: no answer from {host}:{port} within {ConnectPhaseTimeout.TotalSeconds:0}s " + + $"(tried {string.Join(", ", allowed)}). The connection attempt was dropped rather than refused, " + + "which is what a firewall or a network segment boundary looks like from here."); + } + catch (SocketException ex) + { + socket.Dispose(); + throw new IOException( + $"LLM endpoint TCP: {host}:{port} refused the connection ({ex.SocketErrorCode}, " + + $"tried {string.Join(", ", allowed)}). Something answered — check the port and that the LLM is listening.", + ex); + } catch { socket.Dispose(); @@ -132,12 +232,20 @@ public static IServiceCollection AddNodePilotAi(this IServiceCollection services UseProxy = true, Proxy = sp.GetRequiredService(), AllowAutoRedirect = false, + // Bounds establishing a connection, TLS handshake included — the one phase the + // ConnectCallback below cannot cover, because it hands back the raw transport + // stream and the handler negotiates TLS on top of it. Without it a handshake that + // stalls (client-certificate demand, SNI mismatch, a middlebox that accepts the + // socket and never speaks TLS) runs against the per-call answer budget instead, + // which operators legitimately set to minutes for slow local models. + ConnectTimeout = LlmConnectGuard.HandshakeTimeout, // L-4: SSRF guard at TCP-connect time. Closes the DNS-rebinding window // between IsCloudMetadataEndpoint (literal-host check at boot) and the // actual outbound connect on every request. NB: with a proxy in the path this // callback sees the proxy endpoint, not the LLM host — see LlmConfiguredProxy // for why that trade-off is accepted here. - ConnectCallback = LlmConnectGuard.ConnectAsync, + ConnectCallback = (ctx, ct) => LlmConnectGuard.ConnectAsync( + ctx, sp.GetRequiredService().CreateLogger(LlmConnectGuard.LoggerCategory), ct), }); services.AddSingleton(); diff --git a/src/NodePilot.Api/Services/SettingsTestProbe.cs b/src/NodePilot.Api/Services/SettingsTestProbe.cs index fd00f3b..de12431 100644 --- a/src/NodePilot.Api/Services/SettingsTestProbe.cs +++ b/src/NodePilot.Api/Services/SettingsTestProbe.cs @@ -114,7 +114,11 @@ public async Task TestLlmAsync(LlmTestProbeRequest requ // shared LlmEndpointGuard — same validation/guard the runtime LLM calls use. Rejects // cloud-metadata / non-http(s) BaseUrls before any connect. var client = _httpFactory.CreateClient(LlmHttpClient.Name); - client.Timeout = TimeSpan.FromSeconds(Math.Min(request.Settings.TimeoutSeconds, 30)); + // Deliberately above the handler's 30 s connect/TLS budget: whichever deadline fires + // first decides the message the operator reads, and the handler's produces a named + // stage ("TLS handshake did not complete") where this one only ever produces + // "the request was canceled due to the configured HttpClient.Timeout". + client.Timeout = TimeSpan.FromSeconds(Math.Min(request.Settings.TimeoutSeconds, 40)); var url = LlmEndpointGuard.ResolveEndpoint(request.Settings.BaseUrl).ApiRoot + "/models"; using var probe = new HttpRequestMessage(HttpMethod.Get, url); @@ -140,10 +144,21 @@ public async Task TestLlmAsync(LlmTestProbeRequest requ { sw.Stop(); _log.LogWarning(ex, "LLM test probe failed against {BaseUrl}.", request.Settings.BaseUrl); + + // The operator only ever sees this string, and the useful half of it lives in the inner + // exception: HttpRequestException's own message is a generic "An error occurred while + // sending the request", while the connect guard's stage name or the certificate + // failure sits one or two levels down. + var innermost = ex; + while (innermost.InnerException is not null) innermost = innermost.InnerException; + var detail = ReferenceEquals(innermost, ex) + ? ex.Message + : $"{ex.Message} → {innermost.Message}"; + return SettingsTestProbeResult.Failure( - $"LLM probe failed: {ex.Message}", + $"LLM probe failed: {detail}", sw.Elapsed.TotalMilliseconds, - ex.GetType().Name); + innermost.GetType().Name); } } diff --git a/src/nodepilot-docs-ui/content/ai-features.md b/src/nodepilot-docs-ui/content/ai-features.md index ad0ca6d..6fff188 100644 --- a/src/nodepilot-docs-ui/content/ai-features.md +++ b/src/nodepilot-docs-ui/content/ai-features.md @@ -170,7 +170,7 @@ Je Profil: | `ApiKey` | API-Schlüssel; für lokale Modelle häufig nicht erforderlich | | `Model` | verwendeter Modellname | | `MaxTokens` | maximale Länge einer Modellantwort | -| `TimeoutSeconds` | maximale Wartezeit auf den Modell-Endpunkt | +| `TimeoutSeconds` | wie lange das Modell für seine Antwort brauchen darf — nicht die Wartezeit auf die Verbindung, die hat eigene, kurze Fristen | | `EnableToolCalling` | erlaubt den Chats, freigegebene lesende Analyse- und Wissensquellen zu verwenden | | `ToolCallMaxDepth` | maximale Anzahl aufeinanderfolgender Tool-Aufrufe pro Frage | @@ -199,6 +199,27 @@ für den Proxy selbst; die Base-URL wird weiterhin beim Speichern und beim Start Änderungen am Proxy wirken ohne Dienstneustart. Einzige Ausnahme ist `System`: Änderungen an den Windows-Proxy-Einstellungen selbst werden erst nach einem Neustart des Dienstes übernommen. +### Wenn der Endpunkt nicht erreichbar ist + +Den Endpunkt zu erreichen und auf eine Antwort zu warten sind zwei getrennte Dinge mit getrennten +Fristen. `TimeoutSeconds` gilt nur für die Antwort des Modells; der Verbindungsaufbau scheitert +unabhängig davon nach wenigen Sekunden. Ein großzügiges Zeitlimit für ein langsames Modell führt +also nicht dazu, dass man bei einem unerreichbaren Endpunkt minutenlang wartet. + +Die Fehlermeldung benennt die Stufe, an der es gescheitert ist: + +| Meldung beginnt mit | Bedeutung | +|---|---| +| `LLM endpoint DNS:` | Der Name konnte nicht aufgelöst werden — falscher Name, falsches Suffix, oder der Namensdienst antwortet nicht. | +| `LLM endpoint TCP:` | Der Rechner war nicht erreichbar. *Abgelehnt* heißt: der Host antwortet, aber auf diesem Port hört nichts. *Keine Antwort* heißt in aller Regel Firewall oder Netzsegment. | +| `LLM endpoint TLS:` | Die Verbindung stand, aber die Verschlüsselung kam nicht zustande — häufig ein verlangtes Client-Zertifikat oder ein Zertifikat, dem der Server nicht vertraut. | +| `accepted the request but sent no answer` | Alles in Ordnung, das Modell hat nur zu lange gebraucht. Hier ist ein höheres `TimeoutSeconds` die richtige Antwort. | + +Ein Zertifikatshinweis, der oft Zeit kostet: NodePilot prüft gegen den Zertifikatsspeicher der +**Maschine**, nicht den des angemeldeten Benutzers. Ein internes Zertifikat, das der Browser auf +dem Arbeitsplatz akzeptiert, muss auf dem NodePilot-Server unter den vertrauenswürdigen +Stammzertifizierungsstellen des Computers liegen. + ### Anfrageformat (ergibt sich aus der Base-URL) OpenAI betreibt zwei Anfrageformate nebeneinander: das klassische **Chat Completions** und die diff --git a/tests/NodePilot.Ai.Tests/LlmConnectGuardTests.cs b/tests/NodePilot.Ai.Tests/LlmConnectGuardTests.cs index 9662129..e19b93a 100644 --- a/tests/NodePilot.Ai.Tests/LlmConnectGuardTests.cs +++ b/tests/NodePilot.Ai.Tests/LlmConnectGuardTests.cs @@ -1,6 +1,7 @@ using System.Net; using System.Reflection; using FluentAssertions; +using Microsoft.Extensions.Logging; using WireMock.RequestBuilders; using WireMock.ResponseBuilders; using WireMock.Server; @@ -104,4 +105,68 @@ public async Task ConnectAsync_LinkLocalEndpoint_IsRejected(string url) (await act.Should().ThrowAsync()) .Which.ToString().Should().Contain("SSRF guard rejected"); } + + // ---- Stage naming: which half of "cannot reach the endpoint" actually failed ------ + + [Fact] + public async Task ConnectAsync_UnresolvableHost_FailsAtTheDnsStage() + { + // .invalid is reserved by RFC 2606 precisely so it can never resolve. + using var client = NewGuardedClient(); + + Func act = () => client.GetAsync("http://nodepilot-endpoint.invalid/v1/models"); + + (await act.Should().ThrowAsync()) + .Which.ToString().Should().Contain("LLM endpoint DNS:", + "name resolution and a dropped connection need different fixes and must not read alike"); + } + + [Fact] + public async Task ConnectAsync_ClosedPort_FailsAtTheTcpStage_AndSaysSomethingAnswered() + { + // Port 1 on loopback: refused, not dropped — the distinction an operator needs, because a + // refusal means the host is reachable and the listener is the problem. + using var client = NewGuardedClient(); + + Func act = () => client.GetAsync("http://127.0.0.1:1/v1/models"); + + var message = (await act.Should().ThrowAsync()).Which.ToString(); + message.Should().Contain("LLM endpoint TCP:"); + message.Should().Contain("refused"); + message.Should().Contain("127.0.0.1", "the addresses actually tried are the diagnostic"); + } + + [Fact] + public async Task ConnectAsync_ReachableEndpoint_LogsTheResolvedAddresses() + { + // The single most useful line when an endpoint "works from my machine" but not from the + // service: a stale AAAA record or a different DNS suffix both look identical from outside. + using var server = WireMockServer.Start(); + server.Given(Request.Create().WithPath("/ping").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200).WithBody("pong")); + + var logger = new CapturingLogger(); + using var client = new HttpClient(new SocketsHttpHandler + { + ConnectTimeout = LlmConnectGuard.HandshakeTimeout, + ConnectCallback = (ctx, ct) => LlmConnectGuard.ConnectAsync(ctx, logger, ct), + }); + + await client.GetAsync($"{server.Url!.TrimEnd('/')}/ping", TestContext.Current.CancellationToken); + + logger.Messages.Should().Contain(m => m.Contains("resolved to")); + logger.Messages.Should().Contain(m => m.Contains("TCP to")); + } + + private sealed class CapturingLogger : ILogger + { + public List Messages { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => Messages.Add(formatter(state, exception)); + } } diff --git a/tests/NodePilot.Ai.Tests/LlmUnreachableDiagnosticsTests.cs b/tests/NodePilot.Ai.Tests/LlmUnreachableDiagnosticsTests.cs new file mode 100644 index 0000000..8bcecdc --- /dev/null +++ b/tests/NodePilot.Ai.Tests/LlmUnreachableDiagnosticsTests.cs @@ -0,0 +1,105 @@ +using System.Net; +using System.Security.Authentication; +using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace NodePilot.Ai.Tests; + +/// +/// Every way of failing to reach an LLM endpoint used to arrive as the same sentence — "LLM +/// endpoint did not respond within {TimeoutSeconds}s" — because one budget covered DNS, TCP, TLS +/// and the model's answer alike. With a profile at 360 s that meant six minutes of silence and +/// then a message that pointed at the model when the real cause was a firewall or an untrusted +/// certificate. +/// +/// These tests pin the split: each stage now fails on its own deadline and says which one it +/// was. They assert the stage naming, not the exact prose — the wording is allowed to +/// improve, the distinction is not allowed to collapse again. +/// +public sealed class LlmUnreachableDiagnosticsTests +{ + private static LlmHttpTransport Transport() => new( + new StubHttpClientFactory(), + new LlmClientConfig( + Endpoint: LlmEndpointGuard.ResolveEndpoint("https://llm.example.intern/v1"), + ApiKey: null, + Model: "test-model", + MaxTokens: 100, + Temperature: null, + TimeoutSeconds: 360), + NullLogger.Instance); + + [Fact] + public void DescribeUnreachable_CertificateRejected_PointsAtTheMachineTrustStore() + { + // The case a corporate endpoint hits: the CA is trusted on the admin's workstation but was + // never rolled out to the host the service runs on. + var ex = new HttpRequestException("An error occurred while sending the request.", + new AuthenticationException("The remote certificate is invalid according to the validation procedure.")); + + var message = Transport().DescribeUnreachable(ex); + + message.Should().Contain("TLS"); + message.Should().Contain("certificate"); + message.Should().Contain("Trusted Root", + "the operator needs to be told where to put the CA, not merely that TLS failed"); + } + + [Fact] + public void DescribeUnreachable_HandshakeTimeout_IsNotReportedAsASlowModel() + { + // SocketsHttpHandler.ConnectTimeout fired. DNS and TCP carry shorter deadlines of their + // own, so this can only be the handshake — an endpoint demanding a client certificate, an + // SNI mismatch, or a middlebox that accepts the socket and never negotiates. + var ex = new HttpRequestException("An error occurred while sending the request.", + new TimeoutException("A connection could not be established within the configured ConnectTimeout.")); + + var message = Transport().DescribeUnreachable(ex); + + message.Should().Contain("TLS handshake"); + message.Should().NotContain("model", + "blaming the model is exactly the misdiagnosis this split exists to prevent"); + } + + [Theory] + [InlineData("LLM endpoint DNS: resolving 'llm.example.intern' did not finish within 15s.")] + [InlineData("LLM endpoint TCP: no answer from llm.example.intern:443 within 15s (tried 10.0.0.5).")] + public void DescribeUnreachable_StageFromTheConnectGuard_IsPassedThroughVerbatim(string guardMessage) + { + // The guard already names its stage; wrapping it in a second, vaguer sentence would only + // bury it. + var ex = new HttpRequestException("An error occurred while sending the request.", + new IOException(guardMessage)); + + Transport().DescribeUnreachable(ex).Should().Be(guardMessage); + } + + [Fact] + public void DescribeUnreachable_UnclassifiedFailure_StillNamesTheEndpoint() + { + var ex = new HttpRequestException("An error occurred while sending the request.", + new IOException("The response ended prematurely.")); + + var message = Transport().DescribeUnreachable(ex); + + message.Should().Contain("unreachable"); + message.Should().Contain("llm.example.intern", "the URL is what the operator checks first"); + message.Should().Contain("The response ended prematurely.", + "the innermost message carries the detail; HttpRequestException's own is generic"); + } + + [Fact] + public void HandshakeTimeout_ExceedsTheConnectPhaseBudget() + { + // The ordering is load-bearing: DescribeUnreachable concludes "TLS" from the fact that a + // ConnectTimeout can only fire once DNS and TCP have already passed their own, shorter + // deadline. If these two ever cross, that inference silently becomes wrong. + LlmConnectGuard.HandshakeTimeout.Should().BeGreaterThan(LlmConnectGuard.ConnectPhaseTimeout); + } + + private sealed class StubHttpClientFactory : IHttpClientFactory + { + public HttpClient CreateClient(string name) => new(); + } +}