diff --git a/internal/ai/ai.go b/internal/ai/ai.go index 01dba50..60cf1c4 100644 --- a/internal/ai/ai.go +++ b/internal/ai/ai.go @@ -47,6 +47,12 @@ type Config struct { // ProviderCompatible and useful for a proxy in front of the others. BaseURL string Model string + // AllowPrivate lets BaseURL be on this machine or its private network. + // + // Set by the caller from who is asking, never read from storage: it is the + // answer to "may this person reach the LAN", and that answer can change after + // the address was saved. See the handler for who gets it and why. + AllowPrivate bool } func (c Config) Configured() bool { @@ -73,13 +79,17 @@ type Client struct { } func New(cfg Config) *Client { + dial := safedial.Client + if cfg.AllowPrivate { + dial = safedial.AllowPrivate + } return &Client{ cfg: cfg, // Generous: a local model on modest hardware can take a while to produce // its first token, and the alternative is a timeout that only ever fires // for the people running their own. - // safedial, not a plain client. `base_url` is typed into a field in the - // interface, so it is a user-supplied address — and a server that fetches a + // safedial by default. `base_url` is typed into a field in the interface, + // so it is a user-supplied address — and a server that fetches a // user-supplied address is a way to ask it to reach things the caller // cannot: the panel next door, a database on the same bridge, the cloud // metadata endpoint. On a homelab that is most of what is worth reaching. @@ -87,7 +97,13 @@ func New(cfg Config) *Client { // The check is on the resolved address rather than on the URL, because a // name answers whatever its owner says — and can answer differently the // second time, after a parse-time check has passed. - http: safedial.Client(120 * time.Second), + // + // And then the exception, which is the whole reason this provider exists: + // a model you run yourself IS on that network. Blocking it made the two + // lines above true and the feature useless — the comment about a local + // model being slow to its first token was written for a case the dialler + // refused to reach. AllowPrivate is granted per request, not per address. + http: dial(120 * time.Second), } } diff --git a/internal/httpapi/integrations_handlers.go b/internal/httpapi/integrations_handlers.go index ae167bd..b55d5e4 100644 --- a/internal/httpapi/integrations_handlers.go +++ b/internal/httpapi/integrations_handlers.go @@ -215,20 +215,31 @@ func (s *Server) handleSetAISettings(w http.ResponseWriter, r *http.Request) { return } - // The address is checked where it is typed, as well as when it is dialled. + // The address is checked where it is typed, as well as when it is dialled. A + // scheme other than http or https is refused outright, for everybody — + // file:// and gopher:// are not providers. // - // safedial refuses a private destination at connect time and is the actual - // wall; this is so the person entering it hears about it now rather than - // discovering it later as a failed summary. A scheme other than http or https - // is refused outright — file:// and gopher:// are not providers. + // The private-network wall has one exception, and it is the point of the + // provider it guards. "OpenAI-compatible (Ollama, LM Studio, …)" means a model + // on this machine or on the desk beside it: 127.0.0.1:11434, 192.168.1.150. + // safedial refuses exactly those, so the address the field suggested in its + // own placeholder was an address the server would not accept. + // + // Admins only, because the wall is there for a real reason: a server that + // fetches a user-supplied address lets the person supplying it reach what + // they cannot reach themselves. An admin on a self-hosted instance already + // owns the host and everything on its network, so this grants them nothing + // new. An invited user is a different person, and for them it stays shut. if url := strings.TrimSpace(req.BaseURL); url != "" { if !strings.HasPrefix(url, "http://") && !strings.HasPrefix(url, "https://") { writeFieldErrors(w, map[string]string{"base_url": "must start with http:// or https://"}) return } - if reason := safedial.CheckURL(url); reason != "" { - writeFieldErrors(w, map[string]string{"base_url": reason}) - return + if !user.IsAdmin { + if reason := safedial.CheckURL(url); reason != "" { + writeFieldErrors(w, map[string]string{"base_url": reason}) + return + } } req.BaseURL = url } @@ -265,11 +276,20 @@ func (s *Server) aiConfig(r *http.Request, userID string) (ai.Config, error) { v, _ := values[key].(string) return v } + // Worked out per request rather than stored beside the address: it answers + // "may this person reach the private network", and somebody who is no longer + // an admin should stop being able to. A flag written at save time would + // outlive the role that granted it. + allowPrivate := false + if u := userFrom(r.Context()); u != nil && u.IsAdmin { + allowPrivate = true + } return ai.Config{ - Provider: ai.Provider(str("provider")), - APIKey: str("api_key"), - BaseURL: str("base_url"), - Model: str("model"), + Provider: ai.Provider(str("provider")), + APIKey: str("api_key"), + BaseURL: str("base_url"), + Model: str("model"), + AllowPrivate: allowPrivate, }, nil } diff --git a/internal/httpapi/integrations_test.go b/internal/httpapi/integrations_test.go index 2f8aace..7e26122 100644 --- a/internal/httpapi/integrations_test.go +++ b/internal/httpapi/integrations_test.go @@ -719,6 +719,86 @@ func TestTheAIProviderSurvivesConnectingSomethingElse(t *testing.T) { } } +// Reported from use as "the AI settings are not saved" — the second time, and a +// different cause than the one above. +// +// The provider is called "OpenAI-kompatibel (Ollama, LM Studio, …)" and the +// address field suggests http://localhost:11434/v1. Both of those live on the +// private network, which safedial refuses — so the address the interface proposed +// was the address the server rejected, and the one provider that exists for +// running your own model could not be configured at all. +// +// It looked like a save that did nothing, because the 422 came back on a field +// with nowhere to show it. Two bugs wearing one coat. +func TestAdminMayPointAIAtAModelOnTheLAN(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + // The three shapes a self-hosted model actually takes: another machine on the + // LAN, the loopback name, and the loopback address. + for _, addr := range []string{ + "http://192.168.1.150:8080/v1", + "http://localhost:11434/v1", + "http://127.0.0.1:1234/v1", + } { + resp, body := ts.do(t, "PUT", "/api/v1/ai/settings", map[string]any{ + "provider": "compatible", "model": "gemma-4-26b-qat", "base_url": addr, + }) + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("save %s: status %d, body %v", addr, resp.StatusCode, body) + } + } + + _, body := ts.do(t, "GET", "/api/v1/ai/settings", nil) + if body["base_url"] != "http://127.0.0.1:1234/v1" { + t.Errorf("base_url = %v after saving, want it to have survived", body["base_url"]) + } + if body["provider"] != "compatible" { + t.Errorf("provider = %v", body["provider"]) + } +} + +// The wall stays up for everybody else. +// +// An admin on a self-hosted instance already owns the host and its network, so a +// private destination grants them nothing they did not have. An invited user is a +// different person, and for them the address field would be a way to find out +// what answers on the host's network. +func TestAnInvitedUserCannotPointAIAtThePrivateNetwork(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + other := ts.newUser(t, "andreas@example.dk", "Andreas") + + resp, body := other.do(t, "PUT", "/api/v1/ai/settings", map[string]any{ + "provider": "compatible", "model": "llama3.1", + "base_url": "http://192.168.1.150:8080/v1", + }) + if resp.StatusCode == http.StatusNoContent { + t.Fatal("an invited user was allowed to point the AI at the private network") + } + fields, _ := body["fields"].(map[string]any) + if fields["base_url"] == nil { + t.Errorf("the refusal did not name the field it was about: %v", body) + } +} + +// A scheme that is not http or https is refused for everybody, admin or not. +func TestAISettingsRefuseANonHTTPAddress(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + resp, body := ts.do(t, "PUT", "/api/v1/ai/settings", map[string]any{ + "provider": "compatible", "model": "llama3.1", "base_url": "file:///etc/passwd", + }) + if resp.StatusCode == http.StatusNoContent { + t.Fatal("file:// was accepted as a provider address") + } + fields, _ := body["fields"].(map[string]any) + if fields["base_url"] == nil { + t.Errorf("no error on base_url: %v", body) + } +} + func TestGmailSettings(t *testing.T) { ts := newTestServer(t) ts.bootstrap(t) diff --git a/internal/safedial/safedial.go b/internal/safedial/safedial.go index 3e07aea..51feb5f 100644 --- a/internal/safedial/safedial.go +++ b/internal/safedial/safedial.go @@ -24,17 +24,58 @@ // lives), the private ranges, unique-local IPv6, and the unspecified address. // Everything else is allowed: this is not an allowlist of the internet, it is a // wall around the inside. +// +// With one named exception, for Tailscale — see tailnets below. package safedial import ( "fmt" "net" "net/http" + "net/netip" "net/url" "syscall" "time" ) +// tailnets are Tailscale's own ranges, and they are not "the inside". +// +// A tailnet is not a network you happen to be on; it is one every member had to +// authenticate to join. A verdande whose host is on one reaches its peers on +// purpose, and the operator who joined it meant to. +// +// Half of this already worked, by luck rather than intent: Go's IsPrivate covers +// RFC1918 and not the carrier-grade NAT range Tailscale borrows, so 100.x +// addresses passed the wall without anybody deciding they should. The IPv6 half +// did not — fd7a:115c:a1e0::/48 sits inside fc00::/7 and was refused. The same +// machine was reachable or not depending on which family its address had, which +// is the kind of difference nobody debugs on purpose. +// +// Naming both makes the allowance deliberate, and writes down its cost: on an +// instance with people you have invited, this is a network they can now ask the +// server to reach. That is the trade, and it is the operator's to make. +var tailnets = []netip.Prefix{ + netip.MustParsePrefix("100.64.0.0/10"), + netip.MustParsePrefix("fd7a:115c:a1e0::/48"), +} + +// IsTailscale reports whether an address belongs to a tailnet. +func IsTailscale(ip net.IP) bool { + addr, ok := netip.AddrFromSlice(ip) + if !ok { + return false + } + // Unmap first: ParseIP gives a v4 address as 4-in-6, and a v4 prefix does not + // contain a v6-shaped address however equal the two look when printed. + addr = addr.Unmap() + for _, n := range tailnets { + if n.Contains(addr) { + return true + } + } + return false +} + // ErrPrivate is returned when a connection was refused for where it pointed. It // deliberately does not say which address it resolved to: the answer is itself // the thing an attacker is fishing for. @@ -46,8 +87,15 @@ func (e *ErrPrivate) Error() string { // Blocked reports whether an address must not be dialled. func Blocked(ip net.IP) bool { - return ip == nil || - ip.IsLoopback() || + if ip == nil { + return true + } + // Before the private check, not after: a tailnet's IPv6 range is unique-local, + // so asking IsPrivate first would refuse it. + if IsTailscale(ip) { + return false + } + return ip.IsLoopback() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || diff --git a/internal/safedial/safedial_test.go b/internal/safedial/safedial_test.go index 7eecd80..d5f2d6a 100644 --- a/internal/safedial/safedial_test.go +++ b/internal/safedial/safedial_test.go @@ -53,3 +53,47 @@ func TestAllowPrivateStillReaches(t *testing.T) { } resp.Body.Close() } + +// Tailscale skal virke, og på begge familier. +// +// IPv4-halvdelen slap allerede igennem, men ved et tilfælde: Gos IsPrivate +// dækker RFC1918 og ikke det CGNAT-område, Tailscale låner. IPv6-halvdelen gjorde +// ikke — fd7a:115c:a1e0::/48 ligger inde i fc00::/7. Samme maskine var altså +// tilgængelig eller ej alt efter, hvilken adressefamilie man ramte den på. +// +// Testen står her for at holde begge dele fast: at 100.x er tilladt med vilje og +// ikke af held, og at fd7a: ikke ryger tilbage bag muren, næste gang nogen rydder +// op i prædikatet. +func TestTailnetIsNotTheInside(t *testing.T) { + for _, s := range []string{ + "100.64.0.0", "100.72.154.85", "100.127.255.255", + "fd7a:115c:a1e0::1", "fd7a:115c:a1e0:ab12:3456:7890:abcd:ef01", + } { + if Blocked(net.ParseIP(s)) { + t.Errorf("Blocked(%s) = true, men det er en tailnet-adresse", s) + } + if !IsTailscale(net.ParseIP(s)) { + t.Errorf("IsTailscale(%s) = false", s) + } + } + + // Undtagelsen skal være så smal, som den er navngivet. Naboerne til området + // er ikke Tailscale, og en anden unique-local-adresse er stadig indenfor. + for _, s := range []string{ + "100.63.255.255", // lige under 100.64.0.0/10 + "100.128.0.0", // lige over + "fd00::1", // unique-local, men ikke Tailscales + "fd7b:115c:a1e0::1", + "192.168.1.150", + } { + if IsTailscale(net.ParseIP(s)) { + t.Errorf("IsTailscale(%s) = true, men den ligger uden for området", s) + } + } + // Og de af dem, der er indenfor, skal stadig være det. + for _, s := range []string{"fd00::1", "fd7b:115c:a1e0::1", "192.168.1.150"} { + if !Blocked(net.ParseIP(s)) { + t.Errorf("Blocked(%s) = false — muren er væk for mere end Tailscale", s) + } + } +} diff --git a/web/src/routes/indstillinger/ai/+page.svelte b/web/src/routes/indstillinger/ai/+page.svelte index 8a515ba..8c1ea0e 100644 --- a/web/src/routes/indstillinger/ai/+page.svelte +++ b/web/src/routes/indstillinger/ai/+page.svelte @@ -21,6 +21,13 @@ { value: 'compatible', label: 'OpenAI-kompatibel (Ollama, LM Studio, …)' } ]; + // Every field error needs somewhere to appear, and this list is the check that + // it does. `base_url` had nowhere: a refused address set `errors`, skipped the + // toast because the object was not empty, and then rendered nothing — so the + // one provider meant for running your own model looked like a save that did + // nothing at all. Anything without a slot is shouted rather than dropped. + const SHOWN = ['provider', 'model', 'base_url']; + // Suggestions, not a closed list: the model field is free text, because a new // model name must not require a new release of this app to be usable. const SUGGESTED_MODEL = { @@ -56,7 +63,8 @@ saved = true; } catch (e) { errors = e.fields ?? {}; - if (!Object.keys(errors).length) app.toast(humanMessage(e)); + const homeless = Object.keys(errors).filter((field) => !SHOWN.includes(field)); + if (!Object.keys(errors).length || homeless.length) app.toast(humanMessage(e)); } finally { saving = false; } @@ -105,6 +113,7 @@ bind:value={settings.model} placeholder={SUGGESTED_MODEL[settings.provider] ?? ''} /> + {#if errors.model}

{errors.model}

{/if} {#if settings.provider === 'compatible'} @@ -116,6 +125,7 @@ bind:value={settings.base_url} placeholder="http://localhost:11434/v1" /> + {#if errors.base_url}

{errors.base_url}

{/if} {/if}