From 0ee878bac7eb27b7a856b5e1b5279081a0512808 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Thu, 6 Aug 2026 15:47:09 +0000 Subject: [PATCH 1/2] Let the AI features go out through a corporate proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LLM HttpClient hard-coded UseProxy=false, so in networks where outbound traffic is only allowed through a proxy the AI features could not reach a cloud endpoint at all — and no configuration key existed to change that. Adds a global Llm:Proxy block with Mode = Off (default, unchanged behaviour) | System (the proxy the service account's OS is configured with) | Custom (own address plus bypass globs), with Basic or Windows-integrated credentials. One block per installation rather than one per profile: the mixed case — cloud model through the proxy, local Ollama direct — is what the bypass list is for, and a single block keeps one handler with one connection pool. The proxy deliberately does not live in the SocketsHttpHandler. That handler is built once per handler lifetime, so binding the config there would have made the whole Llm settings section restart-required, the way RestApi is. Instead LlmConfiguredProxy implements IWebProxy and resolves IOptionsMonitor per request, which keeps the section hot-reloadable including the Enabled kill-switch. In Off mode it bypasses every destination, making "no proxy configured" byte-for-byte the direct connection this client made before. Note the security trade-off, documented at the type and in the reference: with a proxy in the path the proxy resolves destination DNS, so the connect-time link-local guard only covers the proxy endpoint. The destination stays protected by the literal BaseUrl check that runs on every save and at boot. No mandatory allow-list as restApi has — the LLM BaseUrl is a single Admin-only value, not a per-step URL assembled from trigger payloads. Proxy rules are validated in LlmProfileValidation, which both AddNodePilotAi and LlmConfigBootValidator run, so a save that is accepted cannot block the next boot. The bypass-glob translation moves to NodePilot.Core.Net.ProxyBypassPattern so the Engine and Ai stacks share it instead of drifting apart. --- CLAUDE.md | 4 +- README.md | 6 + docs/ai-features.md | 37 ++- docs/claude-reference.md | 8 +- src/NodePilot.Ai/LlmConfiguredProxy.cs | 180 ++++++++++++++ src/NodePilot.Ai/LlmOptions.cs | 6 + src/NodePilot.Ai/LlmProfileValidation.cs | 63 +++++ src/NodePilot.Ai/LlmProxyOptions.cs | 76 ++++++ .../LlmServiceCollectionExtensions.cs | 27 ++- .../Configuration/SettingsSchema.cs | 5 +- .../Configuration/SettingsSections.cs | 63 +++++ .../Validators/LlmConfigBootValidator.cs | 5 + .../Dtos/Settings/LlmSettingsDto.cs | 98 +++++++- .../Hosting/SecurityHardeningWarnings.cs | 5 + src/NodePilot.Api/appsettings.json | 14 +- src/NodePilot.Core/Net/ProxyBypassPattern.cs | 32 +++ .../Security/RestApiHttpClientProvider.cs | 19 +- src/nodepilot-docs-ui/content/ai-features.md | 25 ++ .../content/security/overview.md | 9 + src/nodepilot-ui/e2e/admin-settings.spec.ts | 4 + .../IntegrationsSection.test.tsx | 72 +++++- .../admin-settings/IntegrationsSection.tsx | 166 ++++++++++++- .../src/i18n/locales/de/adminSettings.json | 14 +- .../src/i18n/locales/en/adminSettings.json | 14 +- .../LlmConfiguredProxyTests.cs | 221 ++++++++++++++++++ .../LlmConnectGuardTests.cs | 12 +- .../LlmProfileValidationTests.cs | 86 +++++++ .../Validators/LlmConfigBootValidatorTests.cs | 45 ++++ .../AdminSettingsControllerSectionTests.cs | 152 +++++++++++- .../Activities/RestApiProxyTests.cs | 13 -- .../Security/ProxyBypassPatternTests.cs | 50 ++++ 31 files changed, 1479 insertions(+), 52 deletions(-) create mode 100644 src/NodePilot.Ai/LlmConfiguredProxy.cs create mode 100644 src/NodePilot.Ai/LlmProxyOptions.cs create mode 100644 src/NodePilot.Core/Net/ProxyBypassPattern.cs create mode 100644 tests/NodePilot.Ai.Tests/LlmConfiguredProxyTests.cs create mode 100644 tests/NodePilot.Engine.Tests/Security/ProxyBypassPatternTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 761e0705..100773d9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -315,6 +315,8 @@ Audit-Codes folgen dem Muster `VERB_NOMEN` und sind **zentral** in `NodePilot.Co Opt-in (`Llm:Enabled=false` default), OpenAI-kompatibler Endpunkt, Rate-Limit 20/min/IP. Drei Helfer + eine Activity; Details: `docs/claude-reference.md` + `docs/ai-features.md`. +**LLM-Proxy:** `Llm:Proxy:Mode` = `Off` (default, Direktverbindung) | `System` (Proxy des Dienstkontos) | `Custom` (`Address` + `BypassList`-Globs), dazu `Username`/`Password` bzw. `UseDefaultCredentials`. Ein Block für die ganze Installation, gilt für alle LLM-Aufrufe inkl. Test-Button. Sitzt bewusst **nicht** im `SocketsHttpHandler`, sondern in `LlmConfiguredProxy : IWebProxy` (liest `IOptionsMonitor` pro Request) — nur deshalb bleibt die Sektion hot-reloadable. Mit Proxy sieht `LlmConnectGuard` nur noch den Proxy-Endpunkt; Details + Begründung: `docs/claude-reference.md`. + **LLM-Profile:** Verbindungen liegen als benannte Profile unter `Llm:Profiles:` (Objekt gekeyt nach unveränderlicher Id, kein Array — Secret-Erhalt matcht per Id und übersteht Rename/Reorder). `Llm:ActiveProfileId` wählt das eine aktive Profil; global bleiben nur diese beiden Keys, alles Verbindungsförmige inkl. `EnableToolCalling`/`ToolCallMaxDepth` sitzt im Profil. Kein „nimm das erste"-Fallback: passt nichts → 503 `LLM_NO_ACTIVE_PROFILE` (Boot läuft trotzdem, nur Warning). Ausgeliefert wird `"Profiles": {}` — ein Profil in der Basis-Config wäre über die UI nie löschbar (additive Provider-Kette), Delete-Versuch → 400 `LLM_PROFILE_NOT_DELETABLE`. **Keine scoped `ILlmClient`-Registrierung** (würde vor dem Action-Gate auflösen); Consumer nehmen `ILlmClientFactory`. - **`POST /api/ai/generate-script`** (Admin/Op, SSE-Streaming — tippt live in Monaco) + **`POST /api/ai/generate-workflow`** (Admin/Op, JSON). @@ -322,7 +324,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`. -- **Hardening:** SSRF-Block (Cloud-Metadata), `UseProxy=false`, Klartext-ApiKey-Warning, Prompt-Injection-Mitigation (Schema-only, User-reviewed Insert). Drift-Schutz: `PromptCatalogDriftTest.cs`. Audit: `AI_*`-Codes. +- **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/README.md b/README.md index 9ce0afdb..fc779111 100644 --- a/README.md +++ b/README.md @@ -1183,6 +1183,12 @@ All settings live in [`src/NodePilot.Api/appsettings.json`](src/NodePilot.Api/ap | `Llm:Profiles::TimeoutSeconds` | `90` | HTTP timeout | | `Llm:Profiles::EnableToolCalling` | `false` | Enable chat read-only tool-calling (function-calling loop). Per profile — reliable function-calling is a property of the model | | `Llm:Profiles::ToolCallMaxDepth` | `6` | Tool-loop depth cap (max LLM rounds with tool calls per turn, 1–10) | +| `Llm:Proxy:Mode` | `Off` | Outbound proxy for every LLM call. `Off` = direct, `System` = the proxy the service account's OS is configured with (incl. its own bypass rules), `Custom` = `Llm:Proxy:Address` | +| `Llm:Proxy:Address` | `""` | Proxy URL, e.g. `http://proxy.corp.local:8080`. Required for `Custom`, ignored otherwise | +| `Llm:Proxy:BypassList` | `[]` | Hosts reached directly, shell globs (`localhost`, `*.corp.local`). `Custom` only — `System` uses the OS bypass rules | +| `Llm:Proxy:Username` | `null` | Proxy Basic-auth user | +| `Llm:Proxy:Password` | `null` | Proxy password; prefer the env var `Llm__Proxy__Password` | +| `Llm:Proxy:UseDefaultCredentials` | `false` | Authenticate to the proxy with the service account's Windows credentials (NTLM/Kerberos) instead of user/password | ### Production deployment (set by the installer) diff --git a/docs/ai-features.md b/docs/ai-features.md index 52cb4862..f953bc8d 100644 --- a/docs/ai-features.md +++ b/docs/ai-features.md @@ -74,6 +74,12 @@ Neu-Eintippen. "MaxTokens": 32768, "TimeoutSeconds": 300 } + }, + "Proxy": { + "Mode": "Custom", + "Address": "http://proxy.firma.local:8080", + "BypassList": ["localhost"], + "UseDefaultCredentials": true } } } @@ -100,8 +106,35 @@ Neu-Eintippen. | `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. | -**Restart erforderlich**: nein — die Sektion ist hot-reloadable. Ein Save in der Admin-UI (inkl. -Profilwechsel) greift beim nächsten Aufruf. +**Outbound-Proxy (`Llm:Proxy:*`):** + +Gilt für **alle** ausgehenden LLM-Aufrufe — Script-/Workflow-Generierung, beide Chats, die +`llmQuery`-Activity und den „Testen"-Button in den Settings. Ein Block pro Installation, nicht pro +Profil: der Fall „Cloud-Profil über den Proxy, lokales Ollama direkt" wird über `BypassList` +gelöst, und ein Handler bedeutet einen Connection-Pool. + +| Key | Default | Erklärung | +|---|---|---| +| `Mode` | `Off` | `Off` = Direktverbindung (Verhalten vor Einführung des Proxys). `System` = der Proxy, mit dem das **Dienstkonto** konfiguriert ist (Windows: WinHTTP/WinINET), inklusive dessen eigener Ausnahmeliste. `Custom` = `Address` unten. | +| `Address` | `""` | Proxy-URL, z. B. `http://proxy.firma.local:8080`. **Pflicht bei `Custom`**, sonst ignoriert. Ein leerer Wert bei `Custom` wird schon beim Speichern abgelehnt, nicht erst beim nächsten Start. | +| `BypassList` | `[]` | Hosts, die am Proxy vorbei erreicht werden. Shell-Globs erlaubt (`localhost`, `*.intern`, `10.0.0.1`). Nur bei `Custom` — bei `System` gilt die Ausnahmeliste des Betriebssystems, weil ein Mischbetrieb die Frage „warum ging das nicht über den Proxy" unbeantwortbar machen würde. | +| `Username` | `null` | Für Proxies mit Basic-Auth. | +| `Password` | `null` | Verschlüsselt gespeichert wie jedes andere Settings-Secret. Klartext in der Config löst eine Startup-Hardening-Warnung aus; **empfohlen: Env-Var `Llm__Proxy__Password`**. | +| `UseDefaultCredentials` | `false` | Authentifiziert mit den Windows-Anmeldedaten des Dienstkontos (NTLM/Kerberos) statt mit `Username`/`Password` — der Normalfall bei domänenintegrierten Unternehmens-Proxies. Gilt für `System` **und** `Custom`. | + +> **Sicherheitshinweis.** Sobald ein Proxy im Pfad liegt, löst **er** das Ziel-DNS auf, nicht mehr +> NodePilot. Der Connect-Zeit-SSRF-Guard (`LlmConnectGuard`) sieht dann nur noch den +> Proxy-Endpunkt; das Ziel ist weiterhin durch die Literal-Prüfung der `BaseUrl` geschützt, die bei +> jedem Speichern und beim Boot läuft. Bewusst **keine** Pflicht-Allow-Liste wie bei `restApi`: die +> LLM-`BaseUrl` ist ein einzelner, Admin-only konfigurierter Wert und keine aus Trigger-Payloads +> zusammengesetzte Per-Step-URL. + +**Restart erforderlich**: nein — die Sektion ist hot-reloadable, inklusive `Llm:Proxy:*`. Ein Save +in der Admin-UI (inkl. Profilwechsel und Proxy-Umstellung) greift beim nächsten Aufruf. Der Proxy +wird pro Request aus der laufenden Konfiguration aufgelöst statt beim Bau des HTTP-Handlers — +genau deshalb bleibt die Sektion hot-reloadable, wo `RestApi` (Proxy fest im Handler) es nicht ist. +Eine Ausnahme bleibt `Mode: System`: Änderungen an den **Windows-Proxy-Einstellungen** selbst +greifen erst nach einem Dienst-Neustart, weil .NET die Systemkonfiguration prozessweit cacht. ### Wire-Dialekt (aus der `BaseUrl` abgeleitet) diff --git a/docs/claude-reference.md b/docs/claude-reference.md index 1de6ebe3..b6ca53d4 100644 --- a/docs/claude-reference.md +++ b/docs/claude-reference.md @@ -230,9 +230,15 @@ Drei opt-in Helfer (Default `Llm:Enabled=false`): - **Nested-DTO-Validierung ist Handarbeit**: `Validator.TryValidateObject` rekursiert nicht in Collection-Elemente, `LlmSettingsDto.Validate` validiert daher jedes Profil explizit und meldet `Profiles[i].Feld`. - **Dynamische ConfigKeys**: Die Llm-Adapter-Keys hängen von den Profil-Ids ab → `DelegateSettingsSectionAdapter` hat dafür einen `Func>`-Overload. +**Outbound-Proxy (`Llm:Proxy:*`)**: `Mode` = `Off` (default, Direktverbindung) | `System` (Proxy des **Dienstkontos**, Windows WinHTTP/WinINET inkl. dessen Bypass-Regeln) | `Custom` (`Address` + `BypassList`-Globs). Dazu `Username`/`Password` bzw. `UseDefaultCredentials` (NTLM/Kerberos, schlägt einen expliziten User). Ein Block pro Installation, nicht pro Profil — der Mischfall „Cloud über Proxy, lokales Ollama direkt" ist genau der Zweck der Bypass-Liste. Doku: [`ai-features.md`](ai-features.md). +- **Warum die Sektion trotzdem hot-reloadable bleibt**: Der Proxy sitzt **nicht** im `SocketsHttpHandler` (der wird einmal pro Handler-Lebensdauer gebaut — deshalb ist `RestApi` restart-pflichtig), sondern in `LlmConfiguredProxy : IWebProxy`, das `IOptionsMonitor.CurrentValue` **pro Request** liest. Der Handler trägt fix `UseProxy = true`; `Mode: Off` beantwortet `IsBypassed` für jedes Ziel mit `true` und ist damit verhaltensgleich mit dem früheren `UseProxy = false`. +- **Sicherheitsgrenze**: Mit Proxy im Pfad löst der Proxy das Ziel-DNS auf → `LlmConnectGuard.ConnectAsync` sieht nur noch den Proxy-Endpunkt, der Connect-Zeit-Schutz gegen Link-Local/Metadata deckt das **Ziel** nicht mehr ab. Bleibt: die Literal-Prüfung der `BaseUrl` in `LlmProfileValidation`, die bei jedem Save *und* beim Boot läuft. Bewusst **ohne** Pflicht-Allow-Liste (anders als `RestApi:AllowedHosts`) — die LLM-`BaseUrl` ist ein einzelner Admin-only-Wert, keine aus Trigger-Payloads gebaute Per-Step-URL. +- **Validierung an einer Stelle**: `LlmProfileValidation.ValidateProxy` (Custom-ohne-Adresse, Nicht-http(s), Metadata-Adresse) wird von `AddNodePilotAi` *und* `LlmConfigBootValidator` gefahren — ein Save, der durchgeht, kann den nächsten Boot nicht blockieren. Die Bypass-Globs teilen sich Engine und Ai über `NodePilot.Core.Net.ProxyBypassPattern`. +- **`Mode: System` cacht**: `HttpClient.DefaultProxy` liest die OS-Konfiguration prozessweit einmal — eine Änderung der Windows-Proxy-Einstellungen greift erst nach Dienst-Neustart. Das ist die einzige nicht-hot-reloadbare Ecke der Sektion. + **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. -- Fresh `SocketsHttpHandler` mit `UseProxy=false` (NICHT der `RestApiHttpClientProvider` — der hat SSRF-Guards die `127.0.0.1:11434` blocken würden) +- 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. - 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/LlmConfiguredProxy.cs b/src/NodePilot.Ai/LlmConfiguredProxy.cs new file mode 100644 index 00000000..d4894a81 --- /dev/null +++ b/src/NodePilot.Ai/LlmConfiguredProxy.cs @@ -0,0 +1,180 @@ +using System.Net; +using Microsoft.Extensions.Options; +using NodePilot.Core.Net; + +namespace NodePilot.Ai; + +/// +/// The the LLM transport's SocketsHttpHandler is built with. It +/// resolves Llm:Proxy:* on every request instead of at handler-construction time, +/// which is the whole point: SocketsHttpHandler owns the connection pool and is created +/// once per handler lifetime, so reading the proxy there would have made the Llm settings +/// section restart-required — the way RestApi is. Going through a live +/// keeps the section hot-reloadable, kill-switch and all. +/// +/// is indistinguishable from the old +/// UseProxy = false: answers true for every +/// destination, so the handler connects directly and LlmConnectGuard still sees the real +/// LLM host. That is what makes "no proxy configured" a genuine no-op rather than a new code +/// path. +/// +/// Security trade-off in the two proxy modes. Once a proxy carries the request, the +/// handler's ConnectCallback is invoked for the proxy endpoint — destination DNS is +/// resolved by the proxy, out of NodePilot's reach. The connect-time link-local/cloud-metadata +/// guard therefore stops covering the destination, which is left to the literal BaseUrl +/// check that runs on every settings save and at boot. +/// Deliberately not countered with a mandatory allow-list the way restApi does it: the LLM +/// BaseUrl is one Admin-only value, not a per-step URL assembled from trigger payloads. +/// +public sealed class LlmConfiguredProxy : IWebProxy +{ + private readonly IOptionsMonitor _options; + + /// + /// Last built custom proxy plus the values it was built from. Rebuilding a + /// (and recompiling its bypass regexes) per request would be wasteful; + /// comparing the source values is cheaper and needs no invalidation callback. A race just + /// builds twice, which is harmless. + /// + private volatile CustomProxyCache? _cache; + + public LlmConfiguredProxy(IOptionsMonitor options) + { + _options = options ?? throw new ArgumentNullException(nameof(options)); + } + + /// + /// Credentials the handler presents when the proxy answers 407. Resolved live, like everything + /// else here. wins over an explicit + /// username because a domain-integrated proxy is the case operators reach for it. + /// + public ICredentials? Credentials + { + get + { + var proxy = CurrentOptions; + return proxy.Mode switch + { + LlmProxyMode.Off => null, + LlmProxyMode.System => proxy.UseDefaultCredentials + ? CredentialCache.DefaultCredentials + : HttpClient.DefaultProxy.Credentials, + LlmProxyMode.Custom => ResolveCustomCredentials(proxy), + _ => null, + }; + } + + // The interface demands a setter; nothing in the HTTP stack assigns it (SocketsHttpHandler + // only reads). Throwing beats a silent no-op that would make a caller believe it had + // overridden the configured credentials. + set => throw new NotSupportedException( + "LLM proxy credentials come from Llm:Proxy:* and cannot be assigned at runtime."); + } + + /// Proxy to use for , or null for a direct connection. + public Uri? GetProxy(Uri destination) + { + ArgumentNullException.ThrowIfNull(destination); + + var proxy = CurrentOptions; + return proxy.Mode switch + { + LlmProxyMode.Off => null, + LlmProxyMode.System => HttpClient.DefaultProxy.GetProxy(destination), + LlmProxyMode.Custom => ResolveCustomProxy(proxy).GetProxy(destination), + _ => null, + }; + } + + /// True when is reached without the proxy. + public bool IsBypassed(Uri destination) + { + ArgumentNullException.ThrowIfNull(destination); + + var proxy = CurrentOptions; + return proxy.Mode switch + { + // Every destination bypasses → byte-for-byte the old UseProxy=false behaviour. + LlmProxyMode.Off => true, + LlmProxyMode.System => HttpClient.DefaultProxy.IsBypassed(destination), + LlmProxyMode.Custom => ResolveCustomProxy(proxy).IsBypassed(destination), + _ => true, + }; + } + + private LlmProxyOptions CurrentOptions => _options.CurrentValue.Proxy ?? new LlmProxyOptions(); + + private static ICredentials? ResolveCustomCredentials(LlmProxyOptions proxy) + { + if (proxy.UseDefaultCredentials) return CredentialCache.DefaultCredentials; + if (string.IsNullOrEmpty(proxy.Username)) return null; + return new NetworkCredential(proxy.Username, proxy.Password ?? ""); + } + + private WebProxy ResolveCustomProxy(LlmProxyOptions proxy) + { + var cached = _cache; + if (cached is not null && cached.Matches(proxy)) return cached.Proxy; + + var address = proxy.Address?.Trim(); + if (string.IsNullOrWhiteSpace(address)) + { + // Rejected by LlmProfileValidation on every save and at boot, so this only fires for a + // hand-edited config picked up by hot-reload. Failing loudly beats silently going + // direct when the operator asked for a proxy. + throw new InvalidOperationException( + $"{LlmProxyOptions.SectionName}:Mode is 'Custom' but {LlmProxyOptions.SectionName}:Address is empty. " + + "Set a proxy URL (e.g. http://proxy.corp.local:8080) or switch the mode to 'Off' or 'System'."); + } + + if (!Uri.TryCreate(address, UriKind.Absolute, out var proxyUri) + || (proxyUri.Scheme != Uri.UriSchemeHttp && proxyUri.Scheme != Uri.UriSchemeHttps)) + { + throw new InvalidOperationException( + $"{LlmProxyOptions.SectionName}:Address '{address}' is not a valid http(s) URL."); + } + + var bypass = (proxy.BypassList ?? new List()) + .Where(v => !string.IsNullOrWhiteSpace(v)) + .Select(v => v.Trim()) + .ToArray(); + + var built = new WebProxy( + proxyUri, + BypassOnLocal: false, + BypassList: bypass.Select(ProxyBypassPattern.ToRegex).ToArray()) + { + Credentials = ResolveCustomCredentials(proxy), + }; + + _cache = new CustomProxyCache(built, address, bypass, proxy.Username, proxy.Password, proxy.UseDefaultCredentials); + return built; + } + + /// + /// Snapshot of the values a cached was built from. Compared field by + /// field rather than via a concatenated signature string so the proxy password does not get a + /// second, longer-lived copy in memory. + /// + private sealed record CustomProxyCache( + WebProxy Proxy, + string Address, + string[] Bypass, + string? Username, + string? Password, + bool UseDefaultCredentials) + { + public bool Matches(LlmProxyOptions options) + { + if (!string.Equals(Address, options.Address?.Trim(), StringComparison.Ordinal)) return false; + if (!string.Equals(Username, options.Username, StringComparison.Ordinal)) return false; + if (!string.Equals(Password, options.Password, StringComparison.Ordinal)) return false; + if (UseDefaultCredentials != options.UseDefaultCredentials) return false; + + var incoming = (options.BypassList ?? new List()) + .Where(v => !string.IsNullOrWhiteSpace(v)) + .Select(v => v.Trim()); + return Bypass.SequenceEqual(incoming, StringComparer.Ordinal); + } + } +} diff --git a/src/NodePilot.Ai/LlmOptions.cs b/src/NodePilot.Ai/LlmOptions.cs index f2e8d758..a91210ad 100644 --- a/src/NodePilot.Ai/LlmOptions.cs +++ b/src/NodePilot.Ai/LlmOptions.cs @@ -61,6 +61,12 @@ public class LlmOptions public Dictionary Profiles { get; set; } = new(StringComparer.OrdinalIgnoreCase); + /// + /// How outbound LLM traffic reaches the network. Global rather than per profile — see + /// . Defaults to no proxy, which is the pre-existing behaviour. + /// + public LlmProxyOptions Proxy { get; set; } = new(); + /// /// Resolves against . Returns false when no /// profile is configured or the active id doesn't exist — callers turn that into a 503 diff --git a/src/NodePilot.Ai/LlmProfileValidation.cs b/src/NodePilot.Ai/LlmProfileValidation.cs index 317ea26e..1b381d99 100644 --- a/src/NodePilot.Ai/LlmProfileValidation.cs +++ b/src/NodePilot.Ai/LlmProfileValidation.cs @@ -51,6 +51,69 @@ public static IReadOnlyList ValidateProfileEndpoints(IConfiguratio return issues; } + /// + /// Rules for the Llm:Proxy:* block. Same Llm:Enabled gate as + /// : an untouched default block must never keep an + /// instance from booting. + /// + /// Checked here rather than only where the proxy is built, so a bad value is rejected by + /// the settings PUT instead of detonating on the next restart — the failure mode + /// RestApi:Proxy still has. + /// + public static IReadOnlyList ValidateProxy(IConfiguration configuration) + { + ArgumentNullException.ThrowIfNull(configuration); + + var issues = new List(); + if (!configuration.GetValue($"{LlmOptions.SectionName}:Enabled")) + return issues; + + var modeKey = $"{LlmProxyOptions.SectionName}:Mode"; + var addressKey = $"{LlmProxyOptions.SectionName}:Address"; + + var rawMode = configuration[modeKey]; + if (string.IsNullOrWhiteSpace(rawMode)) return issues; + + if (!Enum.TryParse(rawMode.Trim(), ignoreCase: true, out var mode)) + { + issues.Add(new ProfileIssue( + modeKey, + $"LLM proxy mode '{rawMode}' is not recognised. Use 'Off', 'System', or 'Custom'.")); + return issues; + } + + if (mode != LlmProxyMode.Custom) return issues; + + var address = configuration[addressKey]?.Trim(); + if (string.IsNullOrWhiteSpace(address)) + { + issues.Add(new ProfileIssue( + addressKey, + "LLM proxy mode is 'Custom' but no proxy address is set. Enter a proxy URL " + + "(e.g. http://proxy.corp.local:8080), or switch the mode to 'Off' or 'System'.")); + return issues; + } + + if (!Uri.TryCreate(address, UriKind.Absolute, out var proxyUri) + || (proxyUri.Scheme != Uri.UriSchemeHttp && proxyUri.Scheme != Uri.UriSchemeHttps)) + { + issues.Add(new ProfileIssue( + addressKey, + $"LLM proxy address '{address}' is not a valid http(s) URL.")); + return issues; + } + + if (LlmEndpointGuard.IsCloudMetadataEndpoint(address)) + { + issues.Add(new ProfileIssue( + addressKey, + $"SECURITY: the LLM proxy address ('{address}') points at a cloud-metadata endpoint. " + + "This range (169.254.0.0/16, metadata.google.internal, metadata.azure.com) is always blocked.")); + } + + return issues; + } + /// /// True when Llm:ActiveProfileId names an existing profile. Read straight from /// configuration so it works on a simulated merged config (settings PUT) as well as at boot. diff --git a/src/NodePilot.Ai/LlmProxyOptions.cs b/src/NodePilot.Ai/LlmProxyOptions.cs new file mode 100644 index 00000000..f86a32e8 --- /dev/null +++ b/src/NodePilot.Ai/LlmProxyOptions.cs @@ -0,0 +1,76 @@ +namespace NodePilot.Ai; + +/// How the LLM transport reaches the outside world. +public enum LlmProxyMode +{ + /// Direct connection, no proxy consulted. Default. + Off = 0, + + /// + /// Use the proxy the operating system is configured with — on Windows the WinHTTP/WinINET + /// settings of the account the NodePilot service runs under, including that configuration's + /// own bypass rules. + /// + System = 1, + + /// Use and the settings next to it. + Custom = 2, +} + +/// +/// HTTP-proxy settings for every outbound LLM call, bound from Llm:Proxy:*. One block for +/// the whole feature rather than one per profile: the "cloud profile through the proxy, local +/// Ollama direct" case is what is for, and a single block keeps one +/// handler with one connection pool. +/// +/// Default is — a NodePilot instance never silently +/// routes model prompts through a proxy nobody asked it to use. Corporate environments with a +/// mandatory outbound proxy set to once and +/// are done. +/// +/// Security note. With a proxy in the path, NodePilot no longer resolves the +/// destination's DNS itself — the proxy does. The connect-time SSRF guard +/// (LlmConnectGuard) therefore only sees the proxy endpoint, and the destination is +/// protected by the literal BaseUrl check that runs on every settings save and at boot. +/// That is proportionate here: the LLM BaseUrl is a single Admin-only setting, not a per-step URL +/// assembled from trigger payloads the way restApi's is. +/// +public class LlmProxyOptions +{ + /// Configuration path of this block (Llm:Proxy). + public const string SectionName = $"{LlmOptions.SectionName}:Proxy"; + + /// Off (default) / System / Custom. See . + public LlmProxyMode Mode { get; set; } = LlmProxyMode.Off; + + /// + /// Proxy URL, e.g. http://proxy.corp.local:8080. Required when + /// is , ignored otherwise. + /// + public string? Address { get; set; } + + /// + /// Hosts that skip the proxy. Accepts shell globs (localhost, *.intern, + /// 10.0.0.1). Only consulted in — in + /// the operating system's own bypass list applies, because + /// mixing the two would make "why did this not go through the proxy" unanswerable. + /// + public List BypassList { get; set; } = new(); + + /// Username for a proxy that wants Basic auth. Empty means no explicit credentials. + public string? Username { get; set; } + + /// + /// Password for . Encrypted at rest like every other settings secret; + /// a plaintext value in the config file raises a startup hardening warning. + /// + public string? Password { get; set; } + + /// + /// Authenticate against the proxy with the service account's own Windows credentials + /// (NTLM/Negotiate) instead of /. The usual + /// setting for a domain-integrated corporate proxy. Applies to both + /// and . + /// + public bool UseDefaultCredentials { get; set; } +} diff --git a/src/NodePilot.Ai/LlmServiceCollectionExtensions.cs b/src/NodePilot.Ai/LlmServiceCollectionExtensions.cs index f614bbb0..53a31530 100644 --- a/src/NodePilot.Ai/LlmServiceCollectionExtensions.cs +++ b/src/NodePilot.Ai/LlmServiceCollectionExtensions.cs @@ -96,10 +96,16 @@ public static IServiceCollection AddNodePilotAi(this IServiceCollection services // instance at all. Same helper the settings boot-validator uses, so an accepted save can // never produce a config that refuses to boot. var enabled = configuration.GetValue($"{LlmOptions.SectionName}:Enabled"); - var endpointIssues = LlmProfileValidation.ValidateProfileEndpoints(configuration); + var endpointIssues = LlmProfileValidation.ValidateProfileEndpoints(configuration) + .Concat(LlmProfileValidation.ValidateProxy(configuration)) + .ToList(); if (endpointIssues.Count > 0) throw new InvalidOperationException(string.Join(" ", endpointIssues.Select(i => i.Message))); + // Singleton: it is handed to the primary handler, which outlives any scope, and it holds + // the cached custom WebProxy. + services.AddSingleton(); + services.AddHttpClient(LlmHttpClient.Name, client => { // The per-request timeout is enforced in OpenAiCompatibleLlmClient via a linked @@ -111,17 +117,26 @@ public static IServiceCollection AddNodePilotAi(this IServiceCollection services // controls the timeout. client.Timeout = Timeout.InfiniteTimeSpan; }) - .ConfigurePrimaryHttpMessageHandler(() => new SocketsHttpHandler + .ConfigurePrimaryHttpMessageHandler(sp => new SocketsHttpHandler { // Local endpoints (Ollama, llama.cpp) speak plaintext HTTP on 127.0.0.1. // Cloud endpoints speak HTTPS — the default SocketsHttpHandler validates that - // normally. No forcing HTTPS, no proxy auto-discovery (local endpoints should - // ignore the Windows system proxy). - UseProxy = false, + // normally. No forcing HTTPS. + // + // Proxying is decided per request by LlmConfiguredProxy from Llm:Proxy:*, NOT + // here: this handler is built once per handler lifetime, so reading the config at + // this point would make the whole Llm settings section restart-required. The + // default (Llm:Proxy:Mode=Off) bypasses every destination, which is exactly the + // direct connection this client made before proxy support existed — no proxy + // auto-discovery unless an operator opts in. + UseProxy = true, + Proxy = sp.GetRequiredService(), AllowAutoRedirect = false, // 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. + // 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, }); diff --git a/src/NodePilot.Api/Configuration/SettingsSchema.cs b/src/NodePilot.Api/Configuration/SettingsSchema.cs index 9a6b3d9a..e1598041 100644 --- a/src/NodePilot.Api/Configuration/SettingsSchema.cs +++ b/src/NodePilot.Api/Configuration/SettingsSchema.cs @@ -61,9 +61,12 @@ public static bool IsUnchangedSecretValue(string? incoming) OptionsType: typeof(LlmOptions), DtoType: typeof(LlmSettingsDto), // '*' matches every profile id — the keys are operator-defined, so the path can't be literal. - SecretFieldPaths: ImmutableArray.Create("Profiles.*.ApiKey"), + SecretFieldPaths: ImmutableArray.Create("Profiles.*.ApiKey", "Proxy.Password"), // Hot-reload: ILlmClientFactory + the controller gates read IOptionsMonitor.CurrentValue // per use, so a Settings-UI save (incl. the Llm:Enabled kill-switch) takes effect without a restart. + // Llm:Proxy:* is live too — LlmConfiguredProxy resolves it per request rather than at + // handler-construction time, which is precisely why this section stayed hot-reloadable + // where RestApi (proxy bound into the handler at boot) could not. IsHotReloadable: true, AuditCode: AuditActions.SettingsLlmUpdated), new SettingsSectionDescriptor( diff --git a/src/NodePilot.Api/Configuration/SettingsSections.cs b/src/NodePilot.Api/Configuration/SettingsSections.cs index 2e429550..e96e745c 100644 --- a/src/NodePilot.Api/Configuration/SettingsSections.cs +++ b/src/NodePilot.Api/Configuration/SettingsSections.cs @@ -427,9 +427,17 @@ private static IReadOnlyList LlmConfigKeys(LlmOptions s) foreach (var field in LlmProfileFieldNames) keys.Add($"Llm:Profiles:{id}:{field}"); } + foreach (var field in LlmProxyFieldNames) + keys.Add($"Llm:Proxy:{field}"); return keys; } + /// The proxy field names, in the order they are persisted. + private static readonly string[] LlmProxyFieldNames = + [ + "Mode", "Address", "BypassList", "Username", "Password", "UseDefaultCredentials", + ]; + /// /// Which configuration source owns a profile besides the runtime overrides file — see /// . Null ⇒ the Settings UI can delete it. @@ -460,8 +468,28 @@ private static IReadOnlyList LlmConfigKeys(LlmOptions s) ManagedBy = LlmProfileManagedBy(configRoot, kv.Key), }) .ToList(), + Proxy = BuildLlmProxyDto(s.Proxy), }; + /// + /// Read projection of Llm:Proxy:*. The password is masked here and nowhere else — the + /// whole section payload is also what SettingsKnowledgeReader hands to the LLM as + /// context, and its safety argument rests on this masking. + /// + private static LlmProxyDto BuildLlmProxyDto(LlmProxyOptions? p) + { + p ??= new LlmProxyOptions(); + return new LlmProxyDto + { + Mode = p.Mode.ToString().ToLowerInvariant(), + Address = p.Address ?? "", + BypassList = new List(p.BypassList ?? new List()), + Username = p.Username, + Password = string.IsNullOrEmpty(p.Password) ? null : SettingsSchema.MaskedSecretDisplay, + UseDefaultCredentials = p.UseDefaultCredentials, + }; + } + private static LlmSettingsDto BuildLlmDtoFromJson(JsonObject? section, IConfigurationRoot configRoot) { section ??= new JsonObject(); @@ -487,11 +515,23 @@ private static LlmSettingsDto BuildLlmDtoFromJson(JsonObject? section, IConfigur } } + var proxyObj = section["Proxy"] as JsonObject ?? new JsonObject(); return new LlmSettingsDto { Enabled = section["Enabled"]?.GetValue() ?? false, ActiveProfileId = section["ActiveProfileId"]?.GetValue() ?? "", Profiles = profiles, + Proxy = new LlmProxyDto + { + // Lower-cased for the DTO exactly like BuildLlmProxyDto does, so both read paths + // hand the UI the same token its mode picker binds to. + Mode = NormalizeProxyMode(proxyObj["Mode"]?.GetValue()).ToLowerInvariant(), + Address = proxyObj["Address"]?.GetValue() ?? "", + BypassList = ReadStringArray(proxyObj, "BypassList"), + Username = proxyObj["Username"]?.GetValue(), + Password = HasNonNullValue(proxyObj, "Password") ? SettingsSchema.MaskedSecretDisplay : null, + UseDefaultCredentials = proxyObj["UseDefaultCredentials"]?.GetValue() ?? false, + }, }; } @@ -522,14 +562,37 @@ private static JsonObject BuildLlmSectionObject( profiles[id] = profile; } + var proxy = new JsonObject + { + // Persisted in the enum's own casing so a hand-read config file matches LlmProxyMode. + ["Mode"] = NormalizeProxyMode(dto.Proxy?.Mode), + ["Address"] = dto.Proxy?.Address?.Trim() ?? "", + ["BypassList"] = ToJsonArray(dto.Proxy?.BypassList ?? new List()), + ["UseDefaultCredentials"] = dto.Proxy?.UseDefaultCredentials ?? false, + }; + WriteOrExplicitNull(proxy, "Username", dto.Proxy?.Username); + WriteSecretField(proxy, "Password", dto.Proxy?.Password, + previousSection?["Proxy"] as JsonObject ?? new JsonObject(), protector); + return new JsonObject { ["Enabled"] = dto.Enabled, ["ActiveProfileId"] = dto.ActiveProfileId.Trim(), ["Profiles"] = profiles, + ["Proxy"] = proxy, }; } + /// + /// Canonical casing for the persisted proxy mode. An unparsable value can't reach here (the + /// DTO's Validate rejects it first), so falling back to Off is a defensive default, not a + /// silent correction of operator input. + /// + private static string NormalizeProxyMode(string? mode) + => Enum.TryParse(mode?.Trim() ?? "", ignoreCase: true, out var parsed) + ? parsed.ToString() + : nameof(LlmProxyMode.Off); + /// /// Refuses a save that drops a profile the runtime overrides file doesn't own. /// diff --git a/src/NodePilot.Api/Configuration/Validators/LlmConfigBootValidator.cs b/src/NodePilot.Api/Configuration/Validators/LlmConfigBootValidator.cs index a864a4ef..7a26cc71 100644 --- a/src/NodePilot.Api/Configuration/Validators/LlmConfigBootValidator.cs +++ b/src/NodePilot.Api/Configuration/Validators/LlmConfigBootValidator.cs @@ -32,6 +32,11 @@ public void Validate(IConfiguration configuration, IList is foreach (var issue in LlmProfileValidation.ValidateProfileEndpoints(configuration)) issues.Add(new BootValidationIssue(Name, BootValidationSeverity.Error, issue.ConfigKey, issue.Message)); + // Same deal for Llm:Proxy:*. A "Custom" mode without an address builds no proxy, so the + // first LLM call after a restart would fail on a value the save could have rejected. + foreach (var issue in LlmProfileValidation.ValidateProxy(configuration)) + issues.Add(new BootValidationIssue(Name, BootValidationSeverity.Error, issue.ConfigKey, issue.Message)); + // Deliberately a Warning, not an Error: the AI features are opt-in, and a half-finished // profile setup must not keep the service from booting. The endpoints answer // 503 LLM_NO_ACTIVE_PROFILE instead. diff --git a/src/NodePilot.Api/Dtos/Settings/LlmSettingsDto.cs b/src/NodePilot.Api/Dtos/Settings/LlmSettingsDto.cs index fc780f33..3265ee76 100644 --- a/src/NodePilot.Api/Dtos/Settings/LlmSettingsDto.cs +++ b/src/NodePilot.Api/Dtos/Settings/LlmSettingsDto.cs @@ -1,4 +1,5 @@ using System.ComponentModel.DataAnnotations; +using NodePilot.Ai; namespace NodePilot.Api.Dtos.Settings; @@ -102,6 +103,80 @@ public sealed class LlmProfileProbeDto public int TimeoutSeconds { get; set; } = 90; } +/// +/// Outbound-proxy settings for every LLM call. Mirrors . +/// One block per installation, not per profile — the "cloud through the proxy, local Ollama +/// direct" case is what is for. +/// +public sealed class LlmProxyDto : IValidatableObject +{ + /// Upper bound on bypass entries. Generous — the point is to stop runaway payloads. + public const int MaxBypassEntries = 128; + + /// + /// off (default, direct connection) / system (the OS proxy of the service + /// account) / custom (). Compared case-insensitively against + /// . + /// + [Required(AllowEmptyStrings = false)] + [StringLength(16)] + public string Mode { get; set; } = nameof(LlmProxyMode.Off).ToLowerInvariant(); + + /// Proxy URL. Required when is custom, ignored otherwise. + [StringLength(2048)] + public string Address { get; set; } = ""; + + /// Host patterns that skip the proxy (shell globs). Only used in custom mode. + public List BypassList { get; set; } = new(); + + [StringLength(255)] + public string? Username { get; set; } + + /// SecretField semantics — "__unchanged__" keeps, plaintext rotates, null/empty clears. + public string? Password { get; set; } + + /// Authenticate against the proxy with the service account's Windows credentials. + public bool UseDefaultCredentials { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (!Enum.TryParse(Mode?.Trim() ?? "", ignoreCase: true, out var mode)) + { + yield return new ValidationResult( + "Proxy mode must be one of 'off', 'system', or 'custom'.", new[] { nameof(Mode) }); + yield break; + } + + // Null-guarded: a literal "BypassList": null in the body nulls the property, and an NRE + // here would turn an operator typo into a 500 instead of a field-level 400. + if ((BypassList?.Count ?? 0) > MaxBypassEntries) + { + yield return new ValidationResult( + $"At most {MaxBypassEntries} proxy bypass entries are supported.", new[] { nameof(BypassList) }); + } + + // Only meaningful for 'custom'; validating it in the other modes would reject a parked + // address the operator kept around while temporarily switching to 'system'. + if (mode != LlmProxyMode.Custom) yield break; + + var address = Address?.Trim() ?? ""; + if (address.Length == 0) + { + yield return new ValidationResult( + "Proxy mode 'custom' requires a proxy address (e.g. http://proxy.corp.local:8080).", + new[] { nameof(Address) }); + yield break; + } + + if (!Uri.TryCreate(address, UriKind.Absolute, out var uri) + || (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps)) + { + yield return new ValidationResult( + $"'{address}' is not a valid http(s) proxy URL.", new[] { nameof(Address) }); + } + } +} + /// /// LLM section DTO for the Admin Settings API. Mirrors — /// the operator-tunable knobs only, the per-feature constants (MaxUpstreamVariables, @@ -122,6 +197,9 @@ public sealed class LlmSettingsDto : IValidatableObject public List Profiles { get; set; } = new(); + /// How outbound LLM traffic reaches the network. Defaults to no proxy. + [Required] public LlmProxyDto Proxy { get; set; } = new(); + /// /// Why this exists: Validator.TryValidateObject — which the generic settings /// adapter calls — does not recurse into collection elements. Without validating each @@ -129,10 +207,28 @@ public sealed class LlmSettingsDto : IValidatableObject /// would be dead metadata. /// /// Member names are reported as Profiles[i].Field so the UI can point at the - /// offending row. + /// offending row. The same applies to the nested object, whose members are + /// reported as Proxy.Field. /// public IEnumerable Validate(ValidationContext validationContext) { + if (Proxy is null) + { + yield return new ValidationResult("Proxy is required.", new[] { nameof(Proxy) }); + } + else + { + var proxyResults = new List(); + Validator.TryValidateObject(Proxy, new ValidationContext(Proxy), proxyResults, validateAllProperties: true); + foreach (var r in proxyResults) + { + var members = r.MemberNames.Any() + ? r.MemberNames.Select(m => $"{nameof(Proxy)}.{m}").ToArray() + : new[] { nameof(Proxy) }; + yield return new ValidationResult(r.ErrorMessage, members); + } + } + if (Profiles.Count > MaxProfiles) { yield return new ValidationResult( diff --git a/src/NodePilot.Api/Hosting/SecurityHardeningWarnings.cs b/src/NodePilot.Api/Hosting/SecurityHardeningWarnings.cs index dfb66ddf..67be7721 100644 --- a/src/NodePilot.Api/Hosting/SecurityHardeningWarnings.cs +++ b/src/NodePilot.Api/Hosting/SecurityHardeningWarnings.cs @@ -95,6 +95,11 @@ public static void LogSecurityHardeningWarnings(IConfiguration configuration, IW "end up in appsettings.json backups and Git history.", name, profile.Key); } + if (!string.IsNullOrWhiteSpace(configuration["Llm:Proxy:Password"])) + Log.Warning("SECURITY: Llm:Proxy:Password appears to be set in configuration. Prefer the environment variable " + + "(Llm__Proxy__Password) or a secrets manager to keep the proxy password out of appsettings.json and " + + "backups. On a domain-integrated proxy, Llm:Proxy:UseDefaultCredentials=true needs no password at all."); + // LDAP plaintext bind is operator error: the boot validator rejects UseSsl=false for // enabled deployments and the adapter refuses the bind unconditionally. Warn anyway so // the mismatch is visible even in unvalidated configurations. diff --git a/src/NodePilot.Api/appsettings.json b/src/NodePilot.Api/appsettings.json index 38f7ccc7..e6ca4686 100644 --- a/src/NodePilot.Api/appsettings.json +++ b/src/NodePilot.Api/appsettings.json @@ -153,7 +153,19 @@ "Llm": { "Enabled": false, "ActiveProfileId": "", - "Profiles": {} + "Profiles": {}, + // Outbound proxy for every LLM call. "Off" (default) connects directly, exactly as this + // client always has. "System" adopts the proxy the service account's OS is configured with — + // the usual answer in corporate networks with a mandatory outbound proxy. "Custom" uses + // Address plus the BypassList globs below. + "Proxy": { + "Mode": "Off", + "Address": "", + "BypassList": [], + "Username": null, + "Password": null, + "UseDefaultCredentials": false + } }, "AiKnowledge": { "Enabled": false, diff --git a/src/NodePilot.Core/Net/ProxyBypassPattern.cs b/src/NodePilot.Core/Net/ProxyBypassPattern.cs new file mode 100644 index 00000000..9be67fed --- /dev/null +++ b/src/NodePilot.Core/Net/ProxyBypassPattern.cs @@ -0,0 +1,32 @@ +using System.Text.RegularExpressions; + +namespace NodePilot.Core.Net; + +/// +/// Translates the operator-friendly proxy bypass patterns NodePilot accepts everywhere +/// (RestApi:Proxy:BypassList, Llm:Proxy:BypassList) into the regex form +/// expects. +/// +/// Lives in Core because two independent outbound stacks need it and neither may +/// reference the other: NodePilot.Engine (restApi activity) and NodePilot.Ai +/// (LLM transport), where the dependency direction is Engine → Ai → Core. +/// +public static class ProxyBypassPattern +{ + /// + /// Convert a host pattern (*.internal, api.corp, 10.0.0.1) to a regex. + /// matches its bypass entries against the full request + /// URI (scheme + host + port + path), not just the host — so the emitted expression + /// anchors on the scheme and wraps the host pattern in optional port/path suffixes. + /// Without that anchoring a bare localhost entry would never match anything. + /// + public static string ToRegex(string pattern) + { + ArgumentNullException.ThrowIfNull(pattern); + + var escaped = Regex.Escape(pattern.Trim()); + // Regex.Escape turns "*" into "\*" — re-interpret as ".*" to support shell globs. + escaped = escaped.Replace("\\*", ".*"); + return $@"^https?://{escaped}(:\d+)?(/.*)?$"; + } +} diff --git a/src/NodePilot.Engine/Security/RestApiHttpClientProvider.cs b/src/NodePilot.Engine/Security/RestApiHttpClientProvider.cs index cc864e3b..d0cd9704 100644 --- a/src/NodePilot.Engine/Security/RestApiHttpClientProvider.cs +++ b/src/NodePilot.Engine/Security/RestApiHttpClientProvider.cs @@ -3,9 +3,9 @@ using System.Net.Sockets; using System.Text; using System.Text.Json; -using System.Text.RegularExpressions; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Options; +using NodePilot.Core.Net; using NodePilot.Engine.Options; namespace NodePilot.Engine.Security; @@ -273,21 +273,6 @@ internal static async ValueTask ConnectWithSsrfGuardAsync( } } - /// - /// Convert a user-friendly host pattern ("*.internal", "api.corp", "10.0.0.1") to a - /// regex suitable for . WebProxy matches patterns - /// against the full request URI (scheme + host + port + path), not just the host, so - /// the emitted regex anchors on scheme and wraps the host pattern with optional - /// port/path suffixes. - /// - internal static string ConvertBypassToRegex(string pattern) - { - var escaped = Regex.Escape(pattern.Trim()); - // Regex.Escape turns "*" into "\*" — re-interpret as ".*" to support shell globs. - escaped = escaped.Replace("\\*", ".*"); - return $@"^https?://{escaped}(:\d+)?(/.*)?$"; - } - private static WebProxy CreateProxy( string address, string[] bypassPatterns, @@ -310,7 +295,7 @@ private static WebProxy CreateProxy( string? username, string? password) { - var regexPatterns = bypassPatterns.Select(ConvertBypassToRegex).ToArray(); + var regexPatterns = bypassPatterns.Select(ProxyBypassPattern.ToRegex).ToArray(); var proxy = new WebProxy(proxyUri, BypassOnLocal: false, BypassList: regexPatterns); if (!string.IsNullOrEmpty(username)) proxy.Credentials = new NetworkCredential(username, password ?? ""); diff --git a/src/nodepilot-docs-ui/content/ai-features.md b/src/nodepilot-docs-ui/content/ai-features.md index af9393ec..342bf053 100644 --- a/src/nodepilot-docs-ui/content/ai-features.md +++ b/src/nodepilot-docs-ui/content/ai-features.md @@ -174,6 +174,31 @@ Je Profil: | `EnableToolCalling` | erlaubt den Chats, freigegebene lesende Analyse- und Wissensquellen zu verwenden | | `ToolCallMaxDepth` | maximale Anzahl aufeinanderfolgender Tool-Aufrufe pro Frage | +### Ausgehender Proxy + +In Unternehmensnetzen ist ausgehender Verkehr oft nur über einen Proxy erlaubt. Die Einstellungen +dafür liegen unter `Llm:Proxy` und gelten für **alle** AI-Aufrufe — beide Chats, die +Script- und Workflow-Generierung, die `llmQuery`-Aktivität und die Verbindungsprüfung in den +Einstellungen. Es ist bewusst ein Block für die gesamte Installation und nicht einer je Profil: +der gemischte Fall — Cloud-Modell über den Proxy, lokales Modell direkt — wird über die +Ausnahmeliste abgebildet. + +| Einstellung | Bedeutung | +|---|---| +| `Mode` | `Off` verbindet direkt (Voreinstellung), `System` übernimmt den Proxy des Dienstkontos samt dessen Ausnahmeregeln, `Custom` verwendet die Adresse unten | +| `Address` | Adresse des Proxys, z. B. `http://proxy.firma.local:8080`; bei `Custom` erforderlich | +| `BypassList` | Hosts, die direkt erreicht werden; Platzhalter erlaubt, etwa `localhost` oder `*.firma.local` | +| `Username` | Benutzername für Proxys mit einfacher Anmeldung | +| `Password` | zugehöriges Kennwort; besser über die Umgebungsvariable `Llm__Proxy__Password` setzen | +| `UseDefaultCredentials` | meldet sich mit den Windows-Anmeldedaten des Dienstkontos am Proxy an — der Normalfall bei domänenintegrierten Proxys | + +Zu beachten: Läuft der Verkehr über einen Proxy, löst dieser die Zieladresse auf. Die zusätzliche +Prüfung, die NodePilot sonst unmittelbar vor dem Verbindungsaufbau vornimmt, greift dann nur noch +für den Proxy selbst; die Base-URL wird weiterhin beim Speichern und beim Start geprüft. + +Ä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. + ### Anfrageformat (ergibt sich aus der Base-URL) OpenAI betreibt zwei Anfrageformate nebeneinander: das klassische **Chat Completions** und die diff --git a/src/nodepilot-docs-ui/content/security/overview.md b/src/nodepilot-docs-ui/content/security/overview.md index 7d1ece64..e1e523d8 100644 --- a/src/nodepilot-docs-ui/content/security/overview.md +++ b/src/nodepilot-docs-ui/content/security/overview.md @@ -45,6 +45,15 @@ Per-IP, Sliding-Window — siehe [Authentifizierung](../api/authentication). `RestApi:Proxy:Enabled` (default `false`). Per-Step-Override via `proxyMode` (`default`/`direct`/`custom`). +## LLM-Proxy + +`Llm:Proxy:Mode` (default `Off`) — getrennt vom REST-API-Proxy, weil AI-Verkehr und +Workflow-Verkehr in Unternehmensnetzen unterschiedlich behandelt werden. `System` übernimmt den +Proxy des Dienstkontos, `Custom` eine eigene Adresse mit Ausnahmeliste. Läuft der Verkehr über +einen Proxy, löst dieser die Zieladresse auf — die Prüfung unmittelbar vor dem Verbindungsaufbau +greift dann nur noch für den Proxy selbst, die Base-URL wird weiterhin beim Speichern und beim +Start geprüft. Details: [AI-Funktionen](../ai-features). + ## Hardening-Flags Die vollständige Liste der Guard-Flags mit Defaults: [Hardening-Flags](./hardening). diff --git a/src/nodepilot-ui/e2e/admin-settings.spec.ts b/src/nodepilot-ui/e2e/admin-settings.spec.ts index f987f1c7..2fb597cc 100644 --- a/src/nodepilot-ui/e2e/admin-settings.spec.ts +++ b/src/nodepilot-ui/e2e/admin-settings.spec.ts @@ -50,6 +50,10 @@ const LLM_PAYLOAD = { enableToolCalling: false, toolCallMaxDepth: 6, managedBy: null, }, ], + proxy: { + mode: 'off', address: '', bypassList: [], username: null, password: null, + useDefaultCredentials: false, + }, }; const RETENTION_PAYLOAD = { executions: { enabled: true, maxAgeDays: 30, intervalMinutes: 60, batchSize: 500, archivePath: null }, diff --git a/src/nodepilot-ui/src/__tests__/components/admin-settings/IntegrationsSection.test.tsx b/src/nodepilot-ui/src/__tests__/components/admin-settings/IntegrationsSection.test.tsx index 30748567..e9dd1861 100644 --- a/src/nodepilot-ui/src/__tests__/components/admin-settings/IntegrationsSection.test.tsx +++ b/src/nodepilot-ui/src/__tests__/components/admin-settings/IntegrationsSection.test.tsx @@ -24,9 +24,15 @@ const llmProfile = (over: Record = {}) => ({ ...over, }); +const llmProxy = (over: Record = {}) => ({ + mode: 'off', address: '', bypassList: [], username: null, password: null, + useDefaultCredentials: false, + ...over, +}); + const llmSnapshot = { sectionPath: 'Llm', - payload: { enabled: false, activeProfileId: 'openai', profiles: [llmProfile()] }, + payload: { enabled: false, activeProfileId: 'openai', profiles: [llmProfile()], proxy: llmProxy() }, etag: '"llm-1"', isHotReloadable: true, effectiveSource: {}, @@ -193,6 +199,70 @@ describe('IntegrationsSection — LLM card', () => { expect(body.Profiles[0].ApiKey).toBeNull(); expect(body.Profiles[0].EnableToolCalling).toBe(false); expect(body.Profiles[0].ToolCallMaxDepth).toBe(4); + // The proxy block always rides along, defaulting to the direct connection. + expect(body.Proxy.Mode).toBe('off'); + expect(body.Proxy.Address).toBe(''); + }); + }); + + it('reveals the proxy fields only in custom mode and serialises them', async () => { + let putBody: unknown = null; + server.use(http.put('/api/admin/settings/Llm', async ({ request }) => { + putBody = await request.json(); + return HttpResponse.json({ ...llmSnapshot, etag: '"llm-2"' }); + })); + + renderSection(); + await waitFor(() => expect(screen.getByDisplayValue('http://127.0.0.1:1234/v1')).toBeInTheDocument()); + + // Off: no address field at all — an inert-but-visible input is what gets filled in and then + // debugged for an hour. + expect(screen.queryByPlaceholderText('http://proxy.firma.local:8080')).not.toBeInTheDocument(); + + const mode = screen.getByLabelText(/Modus|^Mode$/i) as HTMLSelectElement; + fireEvent.change(mode, { target: { value: 'system' } }); + // System mode takes the OS configuration — still no address field. + expect(screen.queryByPlaceholderText('http://proxy.firma.local:8080')).not.toBeInTheDocument(); + + fireEvent.change(mode, { target: { value: 'custom' } }); + const address = await screen.findByPlaceholderText('http://proxy.firma.local:8080'); + fireEvent.change(address, { target: { value: 'http://proxy.corp.local:8080' } }); + + clickLlmSave(); + + await waitFor(() => { + + const body = putBody as any; + expect(body.Proxy.Mode).toBe('custom'); + expect(body.Proxy.Address).toBe('http://proxy.corp.local:8080'); + }); + }); + + it('sends __unchanged__ for a stored proxy password the operator did not retype', async () => { + let putBody: unknown = null; + server.use( + http.get('/api/admin/settings/Llm', () => HttpResponse.json({ + ...llmSnapshot, + payload: { + ...llmSnapshot.payload, + proxy: llmProxy({ mode: 'custom', address: 'http://proxy.corp.local:8080', username: 'svc', password: '********' }), + }, + })), + http.put('/api/admin/settings/Llm', async ({ request }) => { + putBody = await request.json(); + return HttpResponse.json({ ...llmSnapshot, etag: '"llm-2"' }); + }), + ); + + renderSection(); + await waitFor(() => expect(screen.getByDisplayValue('http://proxy.corp.local:8080')).toBeInTheDocument()); + clickLlmSave(); + + await waitFor(() => { + + const body = putBody as any; + expect(body.Proxy.Password).toBe('__unchanged__'); + expect(body.Proxy.Username).toBe('svc'); }); }); diff --git a/src/nodepilot-ui/src/components/admin-settings/IntegrationsSection.tsx b/src/nodepilot-ui/src/components/admin-settings/IntegrationsSection.tsx index d8aff8dd..dfd647e4 100644 --- a/src/nodepilot-ui/src/components/admin-settings/IntegrationsSection.tsx +++ b/src/nodepilot-ui/src/components/admin-settings/IntegrationsSection.tsx @@ -11,7 +11,13 @@ import { SecretField, serializeSecretField, type SecretFieldMode } from './Secre import { EnvOverrideBadge } from './EnvOverrideBadge'; import { EtagConflictDialog } from './EtagConflictDialog'; import { TestProbeModal } from './TestProbeModal'; -import { HotReloadHint } from './SectionFormHelpers'; +import { + GroupHeading, + HotReloadHint, + StringListEditor, + TextInput, + Toggle, +} from './SectionFormHelpers'; type SmtpDto = { host: string; @@ -40,10 +46,28 @@ type LlmProfileDto = { managedBy: string | null; }; +/** Mirrors `NodePilot.Ai.LlmProxyMode` — the API sends and accepts these lower-cased. */ +type LlmProxyMode = 'off' | 'system' | 'custom'; + +type LlmProxyDto = { + mode: LlmProxyMode; + address: string; + bypassList: string[]; + username: string | null; + password: string | null; + useDefaultCredentials: boolean; +}; + type LlmDto = { enabled: boolean; activeProfileId: string; profiles: LlmProfileDto[]; + /** One block for the whole feature, not per profile — bypass entries cover the mixed case. */ + proxy: LlmProxyDto; +}; + +const EMPTY_PROXY: LlmProxyDto = { + mode: 'off', address: '', bypassList: [], username: null, password: null, useDefaultCredentials: false, }; /** Draft secret state per profile id — SecretField is stateless, the parent owns mode+value. */ @@ -289,17 +313,22 @@ function LlmCard() { queryFn: () => adminSettings.getSection('Llm'), }); - const [form, setForm] = useState({ enabled: false, activeProfileId: '', profiles: [] }); + const [form, setForm] = useState({ + enabled: false, activeProfileId: '', profiles: [], proxy: EMPTY_PROXY, + }); const [secrets, setSecrets] = useState>({}); + // The proxy password is section-level, so it can't live in the profile-keyed map above. + const [proxySecret, setProxySecret] = useState({ mode: 'change', value: '' }); const [selectedId, setSelectedId] = useState(''); useEffect(() => { if (!data) return; - setForm(data.payload); + setForm({ ...data.payload, proxy: data.payload.proxy ?? EMPTY_PROXY }); // One secret draft per profile: a fresh snapshot means every pending key edit is stale. setSecrets(Object.fromEntries(data.payload.profiles.map((p) => [ p.id, { mode: p.apiKey ? 'keep' : 'change', value: '' } satisfies SecretDraft, ]))); + setProxySecret({ mode: data.payload.proxy?.password ? 'keep' : 'change', value: '' }); setSelectedId((current) => data.payload.profiles.some((p) => p.id === current) ? current @@ -369,6 +398,14 @@ function LlmCard() { Enabled: form.enabled, ActiveProfileId: form.activeProfileId, Profiles: form.profiles.map(buildProfilePayload), + Proxy: { + Mode: form.proxy.mode, + Address: form.proxy.address, + BypassList: form.proxy.bypassList, + Username: form.proxy.username, + Password: serializeSecretField(proxySecret.mode, proxySecret.value), + UseDefaultCredentials: form.proxy.useDefaultCredentials, + }, }); const activeProfileMissing = form.enabled @@ -492,6 +529,16 @@ function LlmCard() { /> )} + setForm((f) => ({ ...f, proxy: { ...f.proxy, ...patch } }))} + onSecretChange={setProxySecret} + /> + saveMutation.mutate()} saving={saveMutation.isPending} @@ -540,6 +587,119 @@ function LlmCard() { ); } +/** + * Outbound proxy for every LLM call. Section-level rather than per profile, so it sits below the + * profile list instead of inside {@link LlmProfileForm}: one connection pool, one setting. + * + * The address/credential fields only appear in `custom` mode — in `system` mode they would be + * inert, and a visible-but-ignored address field is exactly the kind of thing that gets filled in + * and then debugged for an hour. + */ +function LlmProxyForm({ + proxy, secret, hasPersistedPassword, effectiveSource, isEnvLocked, onPatch, onSecretChange, +}: Readonly<{ + proxy: LlmProxyDto; + secret: SecretDraft; + hasPersistedPassword: boolean; + effectiveSource: Record; + isEnvLocked: (key: string) => boolean; + onPatch: (patch: Partial) => void; + onSecretChange: (draft: SecretDraft) => void; +}>) { + const { t } = useTranslation(['adminSettings', 'common']); + + return ( + <> + {t('adminSettings:integrations.proxy')} + +
+ + +

+ {proxy.mode === 'system' + ? t('adminSettings:integrations.proxyModeSystemHint') + : t('adminSettings:integrations.proxyModeHint')} +

+
+ + {proxy.mode !== 'off' && ( + <> + onPatch({ useDefaultCredentials: v })} + configKey="Llm:Proxy:UseDefaultCredentials" + effectiveSource={effectiveSource} + isEnvLocked={isEnvLocked} + hint={t('adminSettings:integrations.proxyUseDefaultCredentialsHint')} + /> + + {proxy.mode === 'custom' && ( +
+ onPatch({ address: v })} + configKey="Llm:Proxy:Address" + effectiveSource={effectiveSource} + isEnvLocked={isEnvLocked} + placeholder="http://proxy.firma.local:8080" + /> + onPatch({ username: v || null })} + configKey="Llm:Proxy:Username" + effectiveSource={effectiveSource} + isEnvLocked={isEnvLocked} + /> + {!proxy.useDefaultCredentials && ( +
+ onSecretChange({ ...secret, mode })} + onValueChange={(value) => onSecretChange({ ...secret, value })} + disabled={isEnvLocked('Llm:Proxy:Password')} + /> + +
+ )} +
+ onPatch({ bypassList: v })} + placeholder="localhost" + /> +

+ {t('adminSettings:integrations.proxyBypassListHint')} +

+
+
+ )} + + )} + + ); +} + /** * The editor for one profile. Split out so the card body stays readable and so remounting on * profile switch (via `key`) resets any uncontrolled input state. diff --git a/src/nodepilot-ui/src/i18n/locales/de/adminSettings.json b/src/nodepilot-ui/src/i18n/locales/de/adminSettings.json index db9031eb..d28b7f69 100644 --- a/src/nodepilot-ui/src/i18n/locales/de/adminSettings.json +++ b/src/nodepilot-ui/src/i18n/locales/de/adminSettings.json @@ -290,7 +290,19 @@ "profileNotDeletable": "In \"{{source}}\" definiert — hier editierbar, entfernen lässt es sich nur in dieser Konfigurationsquelle.", "noProfiles": "Noch kein LLM-Profil. Lege eines an und wähle es aus, um die KI-Features zu nutzen.", "newProfileName": "Neues Profil", - "activeProfileRequired": "Wähle ein aktives Profil oder deaktiviere die LLM-Integration." + "activeProfileRequired": "Wähle ein aktives Profil oder deaktiviere die LLM-Integration.", + "proxy": "Outbound-Proxy", + "proxyMode": "Modus", + "proxyModeOff": "Kein Proxy (Direktverbindung)", + "proxyModeSystem": "System-Proxy verwenden", + "proxyModeCustom": "Eigener Proxy", + "proxyModeHint": "Gilt für alle KI-Aufrufe — Chat, Generierung und die llmQuery-Activity. Die Änderung greift sofort; zum Testen erst speichern, dann den Testen-Button nutzen.", + "proxyModeSystemHint": "Nutzt die Proxy-Einstellungen des Dienstkontos, unter dem NodePilot läuft (Windows: WinHTTP/WinINET) — inklusive dessen eigener Ausnahmeliste. Änderungen an den Windows-Einstellungen greifen erst nach einem Neustart des Dienstes.", + "proxyAddress": "Adresse", + "proxyBypassList": "Ausnahmen (kein Proxy)", + "proxyBypassListHint": "Hosts, die direkt erreicht werden. Platzhalter erlaubt, z. B. localhost oder *.firma.local — so bleibt ein lokales Ollama am Proxy vorbei erreichbar.", + "proxyUseDefaultCredentials": "Windows-Anmeldedaten des Dienstkontos verwenden", + "proxyUseDefaultCredentialsHint": "Für domänenintegrierte Proxies mit NTLM/Kerberos. Ersetzt Benutzername und Passwort." }, "retention": { "auditLogCardTitle": "Audit-Log", diff --git a/src/nodepilot-ui/src/i18n/locales/en/adminSettings.json b/src/nodepilot-ui/src/i18n/locales/en/adminSettings.json index 5161b0ca..1259aadc 100644 --- a/src/nodepilot-ui/src/i18n/locales/en/adminSettings.json +++ b/src/nodepilot-ui/src/i18n/locales/en/adminSettings.json @@ -290,7 +290,19 @@ "profileNotDeletable": "Defined in \"{{source}}\" — editable here, but it can only be removed in that configuration source.", "noProfiles": "No LLM profile yet. Add one and select it to use the AI features.", "newProfileName": "New profile", - "activeProfileRequired": "Select an active profile, or turn the LLM integration off." + "activeProfileRequired": "Select an active profile, or turn the LLM integration off.", + "proxy": "Outbound proxy", + "proxyMode": "Mode", + "proxyModeOff": "No proxy (direct connection)", + "proxyModeSystem": "Use the system proxy", + "proxyModeCustom": "Custom proxy", + "proxyModeHint": "Applies to every AI call — chat, generation, and the llmQuery activity. Changes take effect immediately; save first, then use the Test button.", + "proxyModeSystemHint": "Uses the proxy configured for the account the NodePilot service runs under (Windows: WinHTTP/WinINET), including its own bypass rules. Changes to the Windows settings take effect after a service restart.", + "proxyAddress": "Address", + "proxyBypassList": "Bypass list (no proxy)", + "proxyBypassListHint": "Hosts reached directly. Wildcards allowed, e.g. localhost or *.corp.local — this is how a local Ollama keeps working alongside a proxy.", + "proxyUseDefaultCredentials": "Use the service account's Windows credentials", + "proxyUseDefaultCredentialsHint": "For domain-integrated proxies using NTLM/Kerberos. Replaces username and password." }, "retention": { "auditLogCardTitle": "Audit Log", diff --git a/tests/NodePilot.Ai.Tests/LlmConfiguredProxyTests.cs b/tests/NodePilot.Ai.Tests/LlmConfiguredProxyTests.cs new file mode 100644 index 00000000..8c3c75cc --- /dev/null +++ b/tests/NodePilot.Ai.Tests/LlmConfiguredProxyTests.cs @@ -0,0 +1,221 @@ +using System.Net; +using FluentAssertions; +using NodePilot.TestCommons; +using Xunit; + +namespace NodePilot.Ai.Tests; + +/// +/// The dynamic behind the LLM HttpClient. The single most important +/// assertion here is the Off case: it must behave exactly like the hard-coded +/// UseProxy = false the handler carried before proxy support existed, because that is what +/// makes "no proxy configured" a genuine no-op for every existing installation. +/// +public sealed class LlmConfiguredProxyTests +{ + private static readonly Uri CloudEndpoint = new("https://api.openai.com/v1/chat/completions"); + private static readonly Uri LocalEndpoint = new("http://localhost:11434/v1/chat/completions"); + + private static (LlmConfiguredProxy Proxy, MutableOptionsMonitor Monitor) Build(LlmProxyOptions proxy) + { + var options = LlmTestOptions.WithProfile(); + options.Proxy = proxy; + var monitor = new MutableOptionsMonitor(options); + return (new LlmConfiguredProxy(monitor), monitor); + } + + [Fact] + public void Off_BypassesEveryDestination_AndOffersNoProxy() + { + var (proxy, _) = Build(new LlmProxyOptions { Mode = LlmProxyMode.Off }); + + proxy.IsBypassed(CloudEndpoint).Should().BeTrue(); + proxy.IsBypassed(LocalEndpoint).Should().BeTrue(); + proxy.GetProxy(CloudEndpoint).Should().BeNull(); + proxy.Credentials.Should().BeNull(); + } + + [Fact] + public void Off_IsTheDefault_WhenNothingIsConfigured() + { + // A fresh LlmOptions must not route anything through a proxy — the upgrade path for every + // existing installation depends on this. + var monitor = new MutableOptionsMonitor(LlmTestOptions.WithProfile()); + var proxy = new LlmConfiguredProxy(monitor); + + proxy.IsBypassed(CloudEndpoint).Should().BeTrue(); + proxy.GetProxy(CloudEndpoint).Should().BeNull(); + } + + [Fact] + public void Custom_RoutesThroughTheConfiguredAddress() + { + var (proxy, _) = Build(new LlmProxyOptions + { + Mode = LlmProxyMode.Custom, + Address = "http://proxy.corp.local:8080", + }); + + proxy.IsBypassed(CloudEndpoint).Should().BeFalse(); + proxy.GetProxy(CloudEndpoint).Should().Be(new Uri("http://proxy.corp.local:8080")); + } + + [Fact] + public void Custom_BypassGlob_KeepsALocalEndpointDirect() + { + // The mixed case the global (rather than per-profile) design relies on: cloud through the + // proxy, local Ollama straight out. + var (proxy, _) = Build(new LlmProxyOptions + { + Mode = LlmProxyMode.Custom, + Address = "http://proxy.corp.local:8080", + BypassList = ["localhost", "*.intern"], + }); + + proxy.IsBypassed(LocalEndpoint).Should().BeTrue(); + proxy.IsBypassed(new Uri("https://llm.intern/v1/chat/completions")).Should().BeTrue(); + proxy.IsBypassed(CloudEndpoint).Should().BeFalse(); + } + + [Fact] + public void Custom_WithUsername_PresentsNetworkCredential() + { + var (proxy, _) = Build(new LlmProxyOptions + { + Mode = LlmProxyMode.Custom, + Address = "http://proxy.corp.local:8080", + Username = "svc-nodepilot", + Password = "s3cret", + }); + + var credential = proxy.Credentials.Should().BeOfType().Subject; + credential.UserName.Should().Be("svc-nodepilot"); + credential.Password.Should().Be("s3cret"); + } + + [Fact] + public void Custom_UseDefaultCredentials_WinsOverAnExplicitUsername() + { + var (proxy, _) = Build(new LlmProxyOptions + { + Mode = LlmProxyMode.Custom, + Address = "http://proxy.corp.local:8080", + Username = "svc-nodepilot", + Password = "s3cret", + UseDefaultCredentials = true, + }); + + proxy.Credentials.Should().BeSameAs(CredentialCache.DefaultCredentials); + } + + [Fact] + public void System_DelegatesToTheProcessDefaultProxy() + { + var (proxy, _) = Build(new LlmProxyOptions { Mode = LlmProxyMode.System }); + + // No assumption about what the host is configured with — only that the answer is the + // OS-derived one rather than NodePilot's own. + proxy.IsBypassed(CloudEndpoint).Should().Be(HttpClient.DefaultProxy.IsBypassed(CloudEndpoint)); + proxy.GetProxy(CloudEndpoint).Should().Be(HttpClient.DefaultProxy.GetProxy(CloudEndpoint)); + } + + [Fact] + public void System_UseDefaultCredentials_PresentsTheServiceAccount() + { + var (proxy, _) = Build(new LlmProxyOptions { Mode = LlmProxyMode.System, UseDefaultCredentials = true }); + + proxy.Credentials.Should().BeSameAs(CredentialCache.DefaultCredentials); + } + + [Fact] + public void ModeChange_TakesEffectWithoutRebuildingTheProxy() + { + // This is the whole reason the proxy is resolved per request instead of at handler + // construction — it is what keeps the Llm settings section hot-reloadable. + var (proxy, monitor) = Build(new LlmProxyOptions { Mode = LlmProxyMode.Off }); + proxy.IsBypassed(CloudEndpoint).Should().BeTrue(); + + var updated = LlmTestOptions.WithProfile(); + updated.Proxy = new LlmProxyOptions + { + Mode = LlmProxyMode.Custom, + Address = "http://proxy.corp.local:8080", + }; + monitor.Set(updated); + + proxy.IsBypassed(CloudEndpoint).Should().BeFalse(); + proxy.GetProxy(CloudEndpoint).Should().Be(new Uri("http://proxy.corp.local:8080")); + } + + [Fact] + public void AddressChange_RebuildsTheCachedProxy() + { + var options = LlmTestOptions.WithProfile(); + options.Proxy = new LlmProxyOptions { Mode = LlmProxyMode.Custom, Address = "http://p1:8080" }; + var monitor = new MutableOptionsMonitor(options); + var proxy = new LlmConfiguredProxy(monitor); + + proxy.GetProxy(CloudEndpoint).Should().Be(new Uri("http://p1:8080")); + + var updated = LlmTestOptions.WithProfile(); + updated.Proxy = new LlmProxyOptions { Mode = LlmProxyMode.Custom, Address = "http://p2:8080" }; + monitor.Set(updated); + + proxy.GetProxy(CloudEndpoint).Should().Be(new Uri("http://p2:8080")); + } + + [Fact] + public void BypassListChange_RebuildsTheCachedProxy() + { + // The cache compares the source values field by field; a changed bypass list must not be + // masked by an unchanged address. + var options = LlmTestOptions.WithProfile(); + options.Proxy = new LlmProxyOptions { Mode = LlmProxyMode.Custom, Address = "http://p1:8080" }; + var monitor = new MutableOptionsMonitor(options); + var proxy = new LlmConfiguredProxy(monitor); + + proxy.IsBypassed(LocalEndpoint).Should().BeFalse(); + + var updated = LlmTestOptions.WithProfile(); + updated.Proxy = new LlmProxyOptions + { + Mode = LlmProxyMode.Custom, + Address = "http://p1:8080", + BypassList = ["localhost"], + }; + monitor.Set(updated); + + proxy.IsBypassed(LocalEndpoint).Should().BeTrue(); + } + + [Fact] + public void Custom_WithoutAddress_ThrowsWithAnActionableMessage() + { + // Rejected by LlmProfileValidation on save and at boot, so this only happens for a + // hand-edited config picked up by hot-reload. Failing loudly beats silently going direct. + var (proxy, _) = Build(new LlmProxyOptions { Mode = LlmProxyMode.Custom, Address = "" }); + + proxy.Invoking(p => p.GetProxy(CloudEndpoint)) + .Should().Throw() + .WithMessage("*Llm:Proxy:Address is empty*"); + } + + [Fact] + public void Custom_WithNonHttpAddress_Throws() + { + var (proxy, _) = Build(new LlmProxyOptions { Mode = LlmProxyMode.Custom, Address = "ftp://proxy:21" }); + + proxy.Invoking(p => p.GetProxy(CloudEndpoint)) + .Should().Throw() + .WithMessage("*not a valid http(s) URL*"); + } + + [Fact] + public void CredentialsSetter_Throws_RatherThanSilentlyIgnoringTheAssignment() + { + var (proxy, _) = Build(new LlmProxyOptions { Mode = LlmProxyMode.Off }); + + proxy.Invoking(p => p.Credentials = new NetworkCredential("a", "b")) + .Should().Throw(); + } +} diff --git a/tests/NodePilot.Ai.Tests/LlmConnectGuardTests.cs b/tests/NodePilot.Ai.Tests/LlmConnectGuardTests.cs index f4cdc3a9..9662129f 100644 --- a/tests/NodePilot.Ai.Tests/LlmConnectGuardTests.cs +++ b/tests/NodePilot.Ai.Tests/LlmConnectGuardTests.cs @@ -5,6 +5,7 @@ using WireMock.ResponseBuilders; using WireMock.Server; using Xunit; +using NodePilot.TestCommons; namespace NodePilot.Ai.Tests; @@ -57,10 +58,19 @@ public void IsLinkLocal_NonLinkLocalAddresses_ReturnFalse(string ip) // ---- End-to-end via the real ConnectCallback ------------------------------------- + /// + /// Mirrors the production handler from + /// so this suite exercises the guard in the shape it actually ships in. Production carries + /// UseProxy = true with a configured ; in + /// — the default asserted here — that proxy bypasses every + /// destination, so the connect goes direct and the callback sees the real host. Building it + /// that way rather than hard-coding UseProxy = false keeps the copy honest. + /// private static HttpClient NewGuardedClient() => new HttpClient(new SocketsHttpHandler { - UseProxy = false, + UseProxy = true, + Proxy = new LlmConfiguredProxy(new StaticOptionsMonitor(LlmTestOptions.WithProfile())), AllowAutoRedirect = false, ConnectCallback = LlmConnectGuard.ConnectAsync, }); diff --git a/tests/NodePilot.Ai.Tests/LlmProfileValidationTests.cs b/tests/NodePilot.Ai.Tests/LlmProfileValidationTests.cs index df347133..9bfeb957 100644 --- a/tests/NodePilot.Ai.Tests/LlmProfileValidationTests.cs +++ b/tests/NodePilot.Ai.Tests/LlmProfileValidationTests.cs @@ -74,6 +74,92 @@ public void ValidateProfileEndpoints_ProfileWithoutBaseUrl_IsSkipped() issues.Should().BeEmpty(); } + [Fact] + public void ValidateProxy_Disabled_ChecksNothing() + { + // Same gate as the profile check: an untouched block must never block a boot. + LlmProfileValidation.ValidateProxy(Config( + ("Llm:Enabled", "false"), + ("Llm:Proxy:Mode", "Custom"))) + .Should().BeEmpty(); + } + + [Fact] + public void ValidateProxy_ModeOffOrSystem_NeedsNoAddress() + { + LlmProfileValidation.ValidateProxy(Config( + ("Llm:Enabled", "true"), ("Llm:Proxy:Mode", "Off"))).Should().BeEmpty(); + LlmProfileValidation.ValidateProxy(Config( + ("Llm:Enabled", "true"), ("Llm:Proxy:Mode", "System"))).Should().BeEmpty(); + } + + [Fact] + public void ValidateProxy_NoModeConfigured_ReturnsNoIssues() + { + LlmProfileValidation.ValidateProxy(Config(("Llm:Enabled", "true"))).Should().BeEmpty(); + } + + [Fact] + public void ValidateProxy_UnknownMode_IsReported() + { + var issues = LlmProfileValidation.ValidateProxy(Config( + ("Llm:Enabled", "true"), + ("Llm:Proxy:Mode", "sometimes"))); + + issues.Should().ContainSingle(); + issues[0].ConfigKey.Should().Be("Llm:Proxy:Mode"); + } + + [Fact] + public void ValidateProxy_CustomWithoutAddress_IsReported() + { + var issues = LlmProfileValidation.ValidateProxy(Config( + ("Llm:Enabled", "true"), + ("Llm:Proxy:Mode", "Custom"))); + + issues.Should().ContainSingle(); + issues[0].ConfigKey.Should().Be("Llm:Proxy:Address"); + issues[0].Message.Should().Contain("no proxy address is set"); + } + + [Theory] + [InlineData("not a url")] + [InlineData("ftp://proxy.corp.local:21")] + [InlineData("proxy.corp.local:8080")] + public void ValidateProxy_CustomWithNonHttpAddress_IsReported(string address) + { + var issues = LlmProfileValidation.ValidateProxy(Config( + ("Llm:Enabled", "true"), + ("Llm:Proxy:Mode", "Custom"), + ("Llm:Proxy:Address", address))); + + issues.Should().ContainSingle(); + issues[0].ConfigKey.Should().Be("Llm:Proxy:Address"); + } + + [Fact] + public void ValidateProxy_CustomWithMetadataAddress_IsReported() + { + // A proxy address is an outbound destination too — the metadata block applies to it. + var issues = LlmProfileValidation.ValidateProxy(Config( + ("Llm:Enabled", "true"), + ("Llm:Proxy:Mode", "Custom"), + ("Llm:Proxy:Address", "http://169.254.169.254:8080"))); + + issues.Should().ContainSingle(); + issues[0].Message.Should().Contain("cloud-metadata"); + } + + [Fact] + public void ValidateProxy_CustomWithValidAddress_ReturnsNoIssues() + { + LlmProfileValidation.ValidateProxy(Config( + ("Llm:Enabled", "true"), + ("Llm:Proxy:Mode", "custom"), // parsed case-insensitively, like the config binder + ("Llm:Proxy:Address", "http://proxy.corp.local:8080"))) + .Should().BeEmpty(); + } + [Theory] [InlineData("a", true)] [InlineData("A", true)] // ids are matched case-insensitively diff --git a/tests/NodePilot.Api.Tests/Configuration/Validators/LlmConfigBootValidatorTests.cs b/tests/NodePilot.Api.Tests/Configuration/Validators/LlmConfigBootValidatorTests.cs index 8a7494e0..ad5a3417 100644 --- a/tests/NodePilot.Api.Tests/Configuration/Validators/LlmConfigBootValidatorTests.cs +++ b/tests/NodePilot.Api.Tests/Configuration/Validators/LlmConfigBootValidatorTests.cs @@ -89,6 +89,51 @@ public void Enabled_WithoutActiveProfile_EmitsWarningNotError() issues.Should().NotContain(i => i.Severity == BootValidationSeverity.Error); } + [Fact] + public void Enabled_ProxyCustomWithoutAddress_EmitsError() + { + // Custom mode with no address builds no proxy, so the first LLM call after a restart would + // fail on a value the save could have rejected outright. + var issues = Run(new() + { + ["Llm:Enabled"] = "true", + ["Llm:ActiveProfileId"] = "a", + ["Llm:Profiles:a:BaseUrl"] = "http://127.0.0.1:1234/v1", + ["Llm:Proxy:Mode"] = "Custom", + }); + + issues.Should().ContainSingle(i => + i.ConfigKey == "Llm:Proxy:Address" && i.Severity == BootValidationSeverity.Error); + } + + [Fact] + public void Enabled_ProxyCustomWithMetadataAddress_EmitsError() + { + var issues = Run(new() + { + ["Llm:Enabled"] = "true", + ["Llm:ActiveProfileId"] = "a", + ["Llm:Profiles:a:BaseUrl"] = "http://127.0.0.1:1234/v1", + ["Llm:Proxy:Mode"] = "Custom", + ["Llm:Proxy:Address"] = "http://169.254.169.254:8080", + }); + + issues.Should().ContainSingle(i => + i.ConfigKey == "Llm:Proxy:Address" && i.Severity == BootValidationSeverity.Error); + } + + [Fact] + public void Enabled_ProxySystem_NoIssues() + { + Run(new() + { + ["Llm:Enabled"] = "true", + ["Llm:ActiveProfileId"] = "a", + ["Llm:Profiles:a:BaseUrl"] = "http://127.0.0.1:1234/v1", + ["Llm:Proxy:Mode"] = "System", + }).Should().BeEmpty(); + } + [Fact] public void Enabled_ActiveProfileIdPointingNowhere_EmitsWarning() { diff --git a/tests/NodePilot.Api.Tests/Controllers/AdminSettingsControllerSectionTests.cs b/tests/NodePilot.Api.Tests/Controllers/AdminSettingsControllerSectionTests.cs index 6220b172..57e974ac 100644 --- a/tests/NodePilot.Api.Tests/Controllers/AdminSettingsControllerSectionTests.cs +++ b/tests/NodePilot.Api.Tests/Controllers/AdminSettingsControllerSectionTests.cs @@ -226,7 +226,12 @@ public async Task PutSection_Retention_OutOfRange_Returns400() "validation must fail before any file is written, otherwise a partially-saved override file could survive"); } - /// A single-profile LLM PUT body. is raw JSON (so a caller can pass null, a string, or the sentinel). + /// + /// A single-profile LLM PUT body. is raw JSON (so a caller can pass + /// null, a string, or the sentinel); is a raw "Proxy": {…}, + /// fragment including its trailing comma, empty by default so the body keeps the shape every + /// pre-proxy caller sends. + /// private static string LlmBody( string baseUrl = "http://localhost:1234/v1", string model = "gpt", @@ -234,11 +239,13 @@ private static string LlmBody( bool enableToolCalling = false, int toolCallMaxDepth = 6, string profileId = "p1", - string activeProfileId = "p1") + string activeProfileId = "p1", + string proxy = "") => $$""" { "Enabled": true, "ActiveProfileId": "{{activeProfileId}}", + {{proxy}} "Profiles": [ { "Id": "{{profileId}}", @@ -755,6 +762,147 @@ public void GetSection_Llm_ConfigKeysCoverEveryProfile() sources.Should().ContainKey("Llm:ActiveProfileId"); sources.Should().ContainKey("Llm:Profiles:alpha:BaseUrl"); sources.Should().ContainKey("Llm:Profiles:beta:ApiKey"); + // The proxy block is section-level, so its keys are present regardless of the profiles. + sources.Should().ContainKey("Llm:Proxy:Mode"); + sources.Should().ContainKey("Llm:Proxy:Address"); + sources.Should().ContainKey("Llm:Proxy:Password"); + } + + [Fact] + public void GetSection_Llm_MasksProxyPassword_AndReportsModeLowerCased() + { + var llm = LlmTestOptions.WithProfile(); + llm.Proxy = new LlmProxyOptions + { + Mode = LlmProxyMode.Custom, + Address = "http://proxy.corp.local:8080", + BypassList = ["localhost"], + Username = "svc", + Password = "proxy-secret", + }; + var (controller, _, _, _) = NewController(initialLlm: llm); + + var result = controller.GetSection("Llm") as OkObjectResult; + var payload = result!.Value!.GetType().GetProperty("Payload")!.GetValue(result.Value) as LlmSettingsDto; + + // Masking matters twice over: this payload is also what SettingsKnowledgeReader hands the + // LLM as context. + payload!.Proxy.Password.Should().Be("********"); + payload.Proxy.Password.Should().NotBe("proxy-secret"); + payload.Proxy.Mode.Should().Be("custom"); + payload.Proxy.Address.Should().Be("http://proxy.corp.local:8080"); + payload.Proxy.BypassList.Should().Equal("localhost"); + payload.Proxy.Username.Should().Be("svc"); + } + + [Fact] + public void GetSection_Llm_NoProxyConfigured_ReportsOff() + { + var (controller, _, _, _) = NewController(initialLlm: LlmTestOptions.WithProfile()); + + var result = controller.GetSection("Llm") as OkObjectResult; + var payload = result!.Value!.GetType().GetProperty("Payload")!.GetValue(result.Value) as LlmSettingsDto; + + payload!.Proxy.Mode.Should().Be("off"); + payload.Proxy.Password.Should().BeNull(); + } + + [Fact] + public async Task PutSection_Llm_PersistsProxy_AndEncryptsItsPassword() + { + var (controller, writer, _, _) = NewController(); + controller.HttpContext.Request.Headers.IfMatch = writer.ComputeSectionEtag("Llm"); + + var body = JsonDocument.Parse(LlmBody(proxy: """ + "Proxy": { "Mode": "custom", "Address": "http://proxy.corp.local:8080", + "BypassList": ["localhost", "*.intern"], "Username": "svc", + "Password": "proxy-secret", "UseDefaultCredentials": false }, + """)).RootElement; + + var result = await controller.PutSection("Llm", body, CancellationToken.None); + + result.Should().BeOfType(); + var fileContent = File.ReadAllText(writer.OverridesPath); + fileContent.Should().NotContain("proxy-secret", + "the proxy password must go through the secret protector like every other settings secret"); + + var proxy = JsonNode.Parse(fileContent)!["Llm"]!["Proxy"]!.AsObject(); + // Persisted in the enum's own casing so a hand-read config file matches LlmProxyMode. + proxy["Mode"]!.GetValue().Should().Be("Custom"); + proxy["Address"]!.GetValue().Should().Be("http://proxy.corp.local:8080"); + proxy["BypassList"]!.AsArray().Select(n => n!.GetValue()).Should().Equal("localhost", "*.intern"); + proxy["Username"]!.GetValue().Should().Be("svc"); + proxy["Password"]!.GetValue().Should().StartWith("enc:v1:"); + } + + [Fact] + public async Task PutSection_Llm_UnchangedProxyPassword_KeepsTheStoredValue() + { + var (controller, writer, _, _) = NewController(); + controller.HttpContext.Request.Headers.IfMatch = writer.ComputeSectionEtag("Llm"); + await controller.PutSection("Llm", JsonDocument.Parse(LlmBody(proxy: """ + "Proxy": { "Mode": "custom", "Address": "http://p:8080", "BypassList": [], + "Username": "svc", "Password": "original", "UseDefaultCredentials": false }, + """)).RootElement, CancellationToken.None); + var stored = JsonNode.Parse(File.ReadAllText(writer.OverridesPath))!["Llm"]!["Proxy"]!["Password"]!.GetValue(); + + controller.HttpContext.Request.Headers.IfMatch = writer.ComputeSectionEtag("Llm"); + await controller.PutSection("Llm", JsonDocument.Parse(LlmBody(proxy: """ + "Proxy": { "Mode": "custom", "Address": "http://p:8080", "BypassList": [], + "Username": "svc", "Password": "__unchanged__", "UseDefaultCredentials": false }, + """)).RootElement, CancellationToken.None); + + JsonNode.Parse(File.ReadAllText(writer.OverridesPath))!["Llm"]!["Proxy"]!["Password"]! + .GetValue().Should().Be(stored); + } + + [Fact] + public async Task PutSection_Llm_ProxyCustomWithoutAddress_Returns400_NoFileWrite() + { + // Two layers reject this — the DTO's own Validate and the boot validator running against + // the simulated merged config. Either way the operator finds out before the restart does. + var (controller, writer, _, _) = NewController(); + controller.HttpContext.Request.Headers.IfMatch = writer.ComputeSectionEtag("Llm"); + + var body = JsonDocument.Parse(LlmBody(proxy: """ + "Proxy": { "Mode": "custom", "Address": "", "BypassList": [], + "Username": null, "Password": null, "UseDefaultCredentials": false }, + """)).RootElement; + + var result = await controller.PutSection("Llm", body, CancellationToken.None); + + result.Should().BeOfType(); + File.Exists(writer.OverridesPath).Should().BeFalse(); + } + + [Fact] + public async Task PutSection_Llm_UnknownProxyMode_Returns400() + { + var (controller, writer, _, _) = NewController(); + controller.HttpContext.Request.Headers.IfMatch = writer.ComputeSectionEtag("Llm"); + + var body = JsonDocument.Parse(LlmBody(proxy: """ + "Proxy": { "Mode": "sometimes", "Address": "", "BypassList": [], + "Username": null, "Password": null, "UseDefaultCredentials": false }, + """)).RootElement; + + (await controller.PutSection("Llm", body, CancellationToken.None)) + .Should().BeOfType(); + } + + [Fact] + public async Task PutSection_Llm_WithoutProxyBlock_DefaultsToOff() + { + // Every pre-existing payload shape omits Proxy entirely; it must stay valid and mean + // "no proxy" rather than failing the [Required] on the nested object. + var (controller, writer, _, _) = NewController(); + controller.HttpContext.Request.Headers.IfMatch = writer.ComputeSectionEtag("Llm"); + + var result = await controller.PutSection("Llm", JsonDocument.Parse(LlmBody()).RootElement, CancellationToken.None); + + result.Should().BeOfType(); + JsonNode.Parse(File.ReadAllText(writer.OverridesPath))!["Llm"]!["Proxy"]!["Mode"]! + .GetValue().Should().Be("Off"); } [Fact] diff --git a/tests/NodePilot.Engine.Tests/Activities/RestApiProxyTests.cs b/tests/NodePilot.Engine.Tests/Activities/RestApiProxyTests.cs index c7545a7b..bb085040 100644 --- a/tests/NodePilot.Engine.Tests/Activities/RestApiProxyTests.cs +++ b/tests/NodePilot.Engine.Tests/Activities/RestApiProxyTests.cs @@ -227,19 +227,6 @@ public void HandlerCache_DifferentSignatures_ReturnDifferentInstances() b.Should().NotBeSameAs(a); } - [Fact] - public void ConvertBypassToRegex_HandlesWildcardsAndLiterals() - { - // WebProxy.BypassList matches patterns against the full URI; the helper wraps the - // host pattern with scheme/port/path suffixes so plain hostnames still bypass. - RestApiHttpClientProvider.ConvertBypassToRegex("*.internal") - .Should().Be(@"^https?://.*\.internal(:\d+)?(/.*)?$"); - RestApiHttpClientProvider.ConvertBypassToRegex("localhost") - .Should().Be(@"^https?://localhost(:\d+)?(/.*)?$"); - RestApiHttpClientProvider.ConvertBypassToRegex("10.0.0.1") - .Should().Be(@"^https?://10\.0\.0\.1(:\d+)?(/.*)?$"); - } - [Fact] public void StepProxyMode_InvalidAddress_Throws() { diff --git a/tests/NodePilot.Engine.Tests/Security/ProxyBypassPatternTests.cs b/tests/NodePilot.Engine.Tests/Security/ProxyBypassPatternTests.cs new file mode 100644 index 00000000..64846b93 --- /dev/null +++ b/tests/NodePilot.Engine.Tests/Security/ProxyBypassPatternTests.cs @@ -0,0 +1,50 @@ +using FluentAssertions; +using NodePilot.Core.Net; +using Xunit; + +namespace NodePilot.Engine.Tests.Security; + +/// +/// The bypass-glob translation shared by RestApi:Proxy:BypassList and +/// Llm:Proxy:BypassList. Lives here rather than in a Core test project because Core +/// has none — the consuming suites cover it. +/// +public class ProxyBypassPatternTests +{ + [Fact] + public void ToRegex_HandlesWildcardsAndLiterals() + { + // WebProxy.BypassList matches patterns against the full URI; the helper wraps the + // host pattern with scheme/port/path suffixes so plain hostnames still bypass. + ProxyBypassPattern.ToRegex("*.internal") + .Should().Be(@"^https?://.*\.internal(:\d+)?(/.*)?$"); + ProxyBypassPattern.ToRegex("localhost") + .Should().Be(@"^https?://localhost(:\d+)?(/.*)?$"); + ProxyBypassPattern.ToRegex("10.0.0.1") + .Should().Be(@"^https?://10\.0\.0\.1(:\d+)?(/.*)?$"); + } + + [Fact] + public void ToRegex_TrimsSurroundingWhitespace() + { + // Operators paste lists; a stray space must not produce a pattern that never matches. + ProxyBypassPattern.ToRegex(" localhost ") + .Should().Be(@"^https?://localhost(:\d+)?(/.*)?$"); + } + + [Theory] + [InlineData("localhost", "http://localhost:11434/v1/models", true)] + [InlineData("localhost", "https://api.openai.com/v1/models", false)] + [InlineData("*.internal", "https://llm.internal/v1/chat/completions", true)] + [InlineData("*.internal", "https://llm.internal.example.com/v1", false)] + [InlineData("127.0.0.1", "http://127.0.0.1:1234/v1", true)] + public void ToRegex_ProducedPattern_MatchesTheIntendedUris(string pattern, string uri, bool expected) + { + var proxy = new System.Net.WebProxy( + new System.Uri("http://proxy.corp.local:8080"), + BypassOnLocal: false, + BypassList: [ProxyBypassPattern.ToRegex(pattern)]); + + proxy.IsBypassed(new System.Uri(uri)).Should().Be(expected); + } +} From 84d33cd9c9b0ed757342e706f93ef7d3b843502a Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:48:25 +0200 Subject: [PATCH 2/2] Pin the LLM probe test to its own card, and point the config page at the proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LLM card renders its heading while the section snapshot is still loading, so `getByRole('button', {name: /test/}).last()` could resolve to SMTP's Test button in the window before the profile form mounts — the test then drove the SMTP probe and timed out waiting for the LLM one. It was already a race on main; the proxy form widened it enough to fail a full-file run. Scoping the lookup to the LLM card removes the ordering assumption instead of adding a wait. Also adds the missing pointer to Llm:Proxy:Mode in the docs-site appsettings overview, which lists the other opt-in connection settings. --- src/nodepilot-docs-ui/content/configuration/appsettings.md | 2 +- src/nodepilot-ui/e2e/admin-settings.spec.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/nodepilot-docs-ui/content/configuration/appsettings.md b/src/nodepilot-docs-ui/content/configuration/appsettings.md index ffe3bec0..abf2d2fb 100644 --- a/src/nodepilot-docs-ui/content/configuration/appsettings.md +++ b/src/nodepilot-docs-ui/content/configuration/appsettings.md @@ -63,7 +63,7 @@ Details: [AD SSO Preview](../enterprise/ldap-windows-sso), [Authentifizierung](. ## KI -`Llm:Enabled` (default `false`) plus mindestens ein Profil unter `Llm:Profiles` und ein `Llm:ActiveProfileId`, das darauf zeigt. Details: [AI-Features](../ai-features). +`Llm:Enabled` (default `false`) plus mindestens ein Profil unter `Llm:Profiles` und ein `Llm:ActiveProfileId`, das darauf zeigt. Ist ausgehender Verkehr nur über einen Proxy erlaubt, kommt `Llm:Proxy:Mode` (default `Off`, sonst `System` oder `Custom`) dazu. Details: [AI-Features](../ai-features). ## Observability diff --git a/src/nodepilot-ui/e2e/admin-settings.spec.ts b/src/nodepilot-ui/e2e/admin-settings.spec.ts index 2fb597cc..3f2930ea 100644 --- a/src/nodepilot-ui/e2e/admin-settings.spec.ts +++ b/src/nodepilot-ui/e2e/admin-settings.spec.ts @@ -190,8 +190,11 @@ test.describe('Admin Settings (Teil 38 + 76)', () => { await openSystemTab(page); await expect(page.getByRole('heading', { name: /llm/i })).toBeVisible({ timeout: 15_000 }); - // The LLM card's Test button lives inside the selected profile's form. - await page.getByRole('button', { name: /^test$|^testen$/i }).last().click(); + // Scope to the LLM card: its heading is already on screen while the section snapshot is + // still loading, so a page-wide `.last()` can resolve to SMTP's Test button in the window + // before the profile form mounts — and then this test would drive the wrong probe. + const llmCard = page.locator('.np-card', { has: page.getByRole('heading', { name: /llm/i }) }); + await llmCard.getByRole('button', { name: /^test$|^testen$/i }).click(); await page.getByRole('button', { name: /run test|test ausführen|test starten/i }).click(); await expect.poll(() => probeHit, { timeout: 10_000 }).toBe(true);