From 11aa4014d0e4e4876f6abb5673e473501d31be0e Mon Sep 17 00:00:00 2001 From: esengine Date: Sat, 10 Oct 2026 00:30:52 -0700 Subject: [PATCH] fix(providers): keep key-carrying provider requests on the configured origin Provider clients took netclient.NewHTTPClient, so a redirect from the configured address was followed by Go's default policy and the key went with it. Add redirectguard.StayOnOrigin, build the probe, provider, System One, balance and doctor clients through endpointclient.New, and guard the default clients of the model listing, balance and System One calls. A refused redirect is a permanent error that names the target host and reaches the user as redirect_refused instead of a network fault retried ten times. --- REASONIX.md | 1 + desktop/frontend-next/src/i18n/en.ts | 1 + desktop/frontend-next/src/i18n/en_kernel.ts | 2 + desktop/frontend-next/src/i18n/kernel.ts | 1 + desktop/frontend-next/src/port/provider.ts | 4 + desktop/frontend-next/src/ui/ModelChoice.tsx | 1 + .../src/ui/provider_check.test.ts | 10 ++ .../frontend-next/src/ui/provider_check.ts | 2 +- internal/assembly/boot/build.go | 9 +- .../assembly/boot/keyed_client_effect_test.go | 99 +++++++++++++++++++ internal/contract/provider/retry_permanent.go | 5 +- .../contract/provider/retry_permanent_test.go | 25 +++++ internal/frontend/serve/provider_check.go | 15 ++- .../frontend/serve/provider_check_decision.go | 3 +- .../frontend/serve/provider_redirect_test.go | 70 +++++++++++++ internal/frontend/serve/providers.go | 14 ++- internal/model/anthropic/anthropic.go | 3 +- internal/model/billing/balance.go | 4 +- .../model/billing/balance_redirect_test.go | 25 +++++ internal/model/catalog/probe_failure.go | 6 ++ internal/model/openai/fetch_models.go | 3 +- .../openai/fetch_models_redirect_test.go | 25 +++++ internal/model/openai/openai.go | 3 +- internal/model/responses/responses.go | 6 +- internal/platform/repair/diagnose.go | 9 +- .../platform/repair/diagnose_redirect_test.go | 34 +++++++ .../safety/endpointclient/endpointclient.go | 21 ++++ .../endpointclient/endpointclient_test.go | 45 +++++++++ .../safety/redirectguard/redirectguard.go | 64 +++++++++++- .../redirectguard/redirectguard_test.go | 75 ++++++++++++++ internal/safety/typesafe/client.go | 4 +- .../safety/typesafe/client_redirect_test.go | 26 +++++ internal/session/control/network.go | 3 +- tools/covergate/baseline.json | 1 + 34 files changed, 598 insertions(+), 21 deletions(-) create mode 100644 desktop/frontend-next/src/ui/provider_check.test.ts create mode 100644 internal/assembly/boot/keyed_client_effect_test.go create mode 100644 internal/frontend/serve/provider_redirect_test.go create mode 100644 internal/model/billing/balance_redirect_test.go create mode 100644 internal/model/openai/fetch_models_redirect_test.go create mode 100644 internal/platform/repair/diagnose_redirect_test.go create mode 100644 internal/safety/endpointclient/endpointclient.go create mode 100644 internal/safety/endpointclient/endpointclient_test.go create mode 100644 internal/safety/typesafe/client_redirect_test.go diff --git a/REASONIX.md b/REASONIX.md index c7e6da0e3a..2547383451 100644 --- a/REASONIX.md +++ b/REASONIX.md @@ -369,6 +369,7 @@ out of how a path is spelled, which cannot tell `internal/auth` from - sensitive: internal/ext/pluginpkg/** - sensitive: internal/base/netclient/** - sensitive: internal/safety/redirectguard/** +- sensitive: internal/safety/endpointclient/** - sensitive: internal/safety/egress/** - sensitive: internal/platform/browser/** - sensitive: internal/platform/computer/** diff --git a/desktop/frontend-next/src/i18n/en.ts b/desktop/frontend-next/src/i18n/en.ts index cbe71b0161..34944b3092 100644 --- a/desktop/frontend-next/src/i18n/en.ts +++ b/desktop/frontend-next/src/i18n/en.ts @@ -528,6 +528,7 @@ export const EN: Record = { "凭据未通过，尚未确认": "Credentials were rejected; availability is still unknown", "遇到限流，尚未确认": "Rate limited; availability is still unknown", "网络异常，尚未确认": "Network error; availability is still unknown", + "地址跳转到了另一个主机，已拒绝，尚未确认": "The address redirected to another host and was refused; availability is still unknown", "验证超时，尚未确认": "Verification timed out; availability is still unknown", "本机服务没有在时限内回答，尚未确认；模型若刚启动可能还在加载，稍后再验证一次": "The local service did not answer in time, so this is unconfirmed. A model that was just started may still be loading; verify again shortly.", "请求被拒绝，尚未确认": "Request rejected; availability is still unknown", diff --git a/desktop/frontend-next/src/i18n/en_kernel.ts b/desktop/frontend-next/src/i18n/en_kernel.ts index 5e284e276b..b53b1476cd 100644 --- a/desktop/frontend-next/src/i18n/en_kernel.ts +++ b/desktop/frontend-next/src/i18n/en_kernel.ts @@ -256,6 +256,8 @@ export const EN_KERNEL: Record = { "The provider errored (HTTP {status}) — nothing you typed is wrong; try again shortly", "该地址在限定时间内没有响应。请检查网络或代理，或稍后重试": "That address did not answer in time. Check the network or proxy, or try again shortly", + "该地址跳转到了 {target}，已拒绝，以免 key 被发到别处。如果信任 {target}，请把服务地址改成 {target}": + "That address redirected to {target} and was refused so the key is not sent elsewhere. If you trust {target}, change the address to {target}", "检查失败，没有具体原因": "The check failed and gave no reason", "无法连接该地址。请检查网络是否通畅，以及地址是否有误": "Cannot reach that address. Check the network, or whether the address has a typo", diff --git a/desktop/frontend-next/src/i18n/kernel.ts b/desktop/frontend-next/src/i18n/kernel.ts index f27e79e875..9b351c5b46 100644 --- a/desktop/frontend-next/src/i18n/kernel.ts +++ b/desktop/frontend-next/src/i18n/kernel.ts @@ -222,6 +222,7 @@ const SAID: Record = { "provider.probe.upstream_error": "服务商返回错误（HTTP {status}），与填写内容无关，请稍后重试", "provider.probe.timeout": "该地址在限定时间内没有响应。请检查网络或代理，或稍后重试", "provider.probe.unreachable": "无法连接该地址。请检查网络是否通畅，以及地址是否有误", + "provider.probe.redirect_refused": "该地址跳转到了 {target}，已拒绝，以免 key 被发到别处。如果信任 {target}，请把服务地址改成 {target}", "provider.probe.failed": "检查失败，没有具体原因", "provider.probe.decision_path_not_found": "地址可以连通，但没有 System One 决策接口。请填写服务的基地址，不要带 /v1/systemone，Reasonix 会自己在后面追加", "provider.probe.decision_not_compatible": "该地址有响应，但返回的不是 System One 的决策结果。请确认这是决策协议服务的基地址", diff --git a/desktop/frontend-next/src/port/provider.ts b/desktop/frontend-next/src/port/provider.ts index d24a499eb4..fa9f879dff 100644 --- a/desktop/frontend-next/src/port/provider.ts +++ b/desktop/frontend-next/src/port/provider.ts @@ -143,6 +143,8 @@ export interface ProviderCheck { ambiguous?: boolean; noProxy?: boolean; code?: string; + // The host a refused redirect pointed at. + target?: string; // Only the numbers the code's sentence needs: `status`, `count`. params?: Record; httpStatus?: number; @@ -160,6 +162,8 @@ export type ProviderModelCheckReason = | "timeout" // Timed out against the machine's own address: the engine did not answer, not the network. | "local_timeout" + // The address redirected to another host; the detail names it. + | "redirect_refused" // The endpoint answered chat and refused a tools array. Established by // sending the same request again without one, not by reading the refusal. | "tools"; diff --git a/desktop/frontend-next/src/ui/ModelChoice.tsx b/desktop/frontend-next/src/ui/ModelChoice.tsx index ea064a2459..59dd008e97 100644 --- a/desktop/frontend-next/src/ui/ModelChoice.tsx +++ b/desktop/frontend-next/src/ui/ModelChoice.tsx @@ -224,6 +224,7 @@ function checkReason(reason?: ProviderModelCheckReason): string { case "network": return "网络异常，尚未确认"; case "timeout": return "验证超时，尚未确认"; case "local_timeout": return "本机服务没有在时限内回答，尚未确认；模型若刚启动可能还在加载，稍后再验证一次"; + case "redirect_refused": return "地址跳转到了另一个主机，已拒绝，尚未确认"; case "rejected": return "请求被拒绝，尚未确认"; default: return "尚未确认"; } diff --git a/desktop/frontend-next/src/ui/provider_check.test.ts b/desktop/frontend-next/src/ui/provider_check.test.ts new file mode 100644 index 0000000000..d2bd610128 --- /dev/null +++ b/desktop/frontend-next/src/ui/provider_check.test.ts @@ -0,0 +1,10 @@ +import { describe, expect, it } from "vitest"; +import { checkFailure } from "./provider_check"; + +describe("checkFailure", () => { + it("names the host a refused redirect pointed at", () => { + const said = checkFailure({ ok: false, code: "provider.probe.redirect_refused", target: "www.example.com" }); + expect(said).toContain("www.example.com"); + expect(said).not.toContain("{target}"); + }); +}); diff --git a/desktop/frontend-next/src/ui/provider_check.ts b/desktop/frontend-next/src/ui/provider_check.ts index c8747e0ba6..f80f47198b 100644 --- a/desktop/frontend-next/src/ui/provider_check.ts +++ b/desktop/frontend-next/src/ui/provider_check.ts @@ -3,7 +3,7 @@ import { say } from "../i18n/kernel"; import type { ProviderCheck } from "../port/port"; export function checkFailure(check: ProviderCheck): string { - const said = say({ code: check.code, params: check.params }) || t("检查失败，没有具体原因"); + const said = say({ code: check.code, params: { ...check.params, ...(check.target ? { target: check.target } : {}) } }) || t("检查失败，没有具体原因"); const status = check.httpStatus ? ` · HTTP ${check.httpStatus}` : ""; const words = check.detail ? ` · ${check.detail}` : ""; return said + status + words; diff --git a/internal/assembly/boot/build.go b/internal/assembly/boot/build.go index 6adb32a747..3928abf972 100644 --- a/internal/assembly/boot/build.go +++ b/internal/assembly/boot/build.go @@ -29,6 +29,7 @@ import ( "reasonix/internal/platform/lsp" "reasonix/internal/runtime/agent" "reasonix/internal/runtime/delegation" + "reasonix/internal/safety/endpointclient" "reasonix/internal/safety/permission" "reasonix/internal/safety/sandbox" "reasonix/internal/session/control" @@ -58,6 +59,7 @@ type builder struct { keep agent.KeepPolicy session sessionRuntime balanceClient *http.Client + keyedClient *http.Client // carries provider keys; refuses cross-origin redirects execProv provider.Provider shell sandbox.Shell prompt promptAssembly @@ -227,6 +229,9 @@ func (b *builder) load() error { if b.balanceClient, err = netclient.NewHTTPClient(b.proxy, netclient.TransportOptions{}); err != nil { return err } + if b.keyedClient, err = endpointclient.New(b.proxy, netclient.TransportOptions{}); err != nil { + return err + } if b.execProv, err = resolveProvider(b.providers.effective, cfg, b.proxy, provider.Selection{Ref: b.model.ref, Effort: opts.EffortOverride}); err != nil { return err } @@ -316,7 +321,7 @@ func (b *builder) wireTools() error { // The full inventory registers for use_capability; the provider-visible surface narrows later. addBuiltins(t.reg, cfg.Tools.Enabled, env.writeRoots, env.bash, env.bashTimeout, env.search, b.stderr, root, b.proxy, env.forbidReadRoots, env.readRoots, env.readPaths, env.sessionGuard, env.managedConfig, opts.FileOverlay, opts.TerminalRunner, env.sessionTemp, b.fileWriteReceipt) bindFileViews(t.reg, cfg.Tools.ChangedFilesProtected()) - addSystemOne(t.reg, cfg.Tools.Enabled, cfg, b.balanceClient) + addSystemOne(t.reg, cfg.Tools.Enabled, cfg, b.keyedClient) addAdvisor(t.reg, cfg, b.proxy, b.sink) b.addBestOf() if cfg.Agent.CodeMode { @@ -541,7 +546,7 @@ func (b *builder) controllerOptions(runner agent.Runner, executor *agent.Agent, Memory: b.prompt.memory, // Read at Close time: freeze chains the extension runtime set onto it. Cleanup: func() { b.cleanup() }, - Balance: opts.BalanceStore.Cache(b.balanceClient, entry.BalanceURL, entry.APIKey()), + Balance: opts.BalanceStore.Cache(b.keyedClient, entry.BalanceURL, entry.APIKey()), Feedback: control.FeedbackOptions{Service: feedbackService(b.roots.Home(), b.proxy), ProviderKind: feedbackProviderKind(entry), Surface: opts.FeedbackSurface}, Jobs: b.session.jobs, TaskStore: opts.TaskStore, diff --git a/internal/assembly/boot/keyed_client_effect_test.go b/internal/assembly/boot/keyed_client_effect_test.go new file mode 100644 index 0000000000..31c45abef6 --- /dev/null +++ b/internal/assembly/boot/keyed_client_effect_test.go @@ -0,0 +1,99 @@ +package boot + +import ( + "context" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + "reasonix/internal/contract/config" + "reasonix/internal/contract/event" + "reasonix/internal/contract/provider" +) + +type systemOneCaller struct{ round atomic.Int32 } + +func (p *systemOneCaller) Name() string { return "boot-keyed-client" } + +func (p *systemOneCaller) Stream(context.Context, provider.Request) (<-chan provider.Chunk, error) { + ch := make(chan provider.Chunk, 2) + if p.round.Add(1) == 1 { + ch <- provider.Chunk{Type: provider.ChunkToolCall, ToolCall: &provider.ToolCall{ + ID: "s1", Name: "system_one", + Arguments: `{"state":"s","questions":{"q":{"type":"noul","instructions":"is it?"}}}`, + }} + } else { + ch <- provider.Chunk{Type: provider.ChunkText, Text: "done"} + } + ch <- provider.Chunk{Type: provider.ChunkDone} + close(ch) + return ch, nil +} + +// The System One call and the balance read both carry the provider key, so a +// redirect off the configured address must not be followed by either. +func TestEffectKeyCarryingBuildClientsRefuseACrossOriginRedirect(t *testing.T) { + var reached atomic.Int32 + elsewhere := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached.Add(1) })) + defer elsewhere.Close() + home := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, elsewhere.URL, http.StatusTemporaryRedirect) + })) + defer home.Close() + + isolateConfigHome(t) + dir := robustTempDir(t) + t.Chdir(dir) + t.Setenv("KEYED_EFFECT_KEY", "secret") + caller := &systemOneCaller{} + provider.Register("typesafe", func(provider.Config) (provider.Provider, error) { return caller, nil }) + provider.Register("boot-keyed-chat", func(provider.Config) (provider.Provider, error) { return caller, nil }) + userCfg := config.UserConfigPath() + if err := os.MkdirAll(filepath.Dir(userCfg), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(userCfg, []byte(` +default_model = "chat/m" + +[agent] +decision_model = "decider/system-one" + +[[providers]] +name = "chat" +kind = "boot-keyed-chat" +models = ["m"] +default = "m" +api_key_env = "KEYED_EFFECT_KEY" +balance_url = "`+home.URL+`/balance" + +[[providers]] +name = "decider" +kind = "typesafe" +balance_url = "`+home.URL+`/balance" +base_url = "`+home.URL+`" +models = ["system-one"] +api_key_env = "KEYED_EFFECT_KEY" +`), 0o600); err != nil { + t.Fatal(err) + } + approveWorkspace(t, dir) + ctrl, err := Build(context.Background(), Options{Sink: event.Discard}) + if err != nil { + t.Fatalf("Build: %v", err) + } + defer ctrl.Close() + if err := ctrl.Run(context.Background(), "ask system one"); err != nil { + t.Fatalf("Run: %v", err) + } + reading := ctrl.Balance(context.Background()) + if reading.Err == nil || !strings.Contains(reading.Err.Error(), "redirect refused") { + t.Fatalf("balance reading = %+v, want a refused redirect", reading) + } + if n := reached.Load(); n != 0 { + t.Fatalf("the redirect target received %d request(s)", n) + } +} diff --git a/internal/contract/provider/retry_permanent.go b/internal/contract/provider/retry_permanent.go index d6a10857a4..e7b72d0a8c 100644 --- a/internal/contract/provider/retry_permanent.go +++ b/internal/contract/provider/retry_permanent.go @@ -9,9 +9,10 @@ import ( // permanentTransportErr reports a transport failure no backoff can fix: the // endpoint refused the connection, its name does not resolve, or its -// certificate does not verify. These fail on the first attempt. +// certificate does not verify, or the error itself says it is permanent. These fail on the first attempt. func permanentTransportErr(err error) bool { - if connectionRefused(err) { + var carried interface{ Permanent() bool } + if connectionRefused(err) || (errors.As(err, &carried) && carried.Permanent()) { return true } var dns *net.DNSError diff --git a/internal/contract/provider/retry_permanent_test.go b/internal/contract/provider/retry_permanent_test.go index ca690a7fc9..499ae66422 100644 --- a/internal/contract/provider/retry_permanent_test.go +++ b/internal/contract/provider/retry_permanent_test.go @@ -6,10 +6,13 @@ import ( "errors" "net" "net/http" + "net/http/httptest" "os" "syscall" "testing" "time" + + "reasonix/internal/safety/redirectguard" ) // 1.x failed a request to an endpoint that refuses connections at once; @@ -39,6 +42,28 @@ func TestSendWithRetryFailsFastWhenTheEndpointRefuses(t *testing.T) { } } +func TestSendWithRetryDoesNotRetryARefusedRedirect(t *testing.T) { + elsewhere := httptest.NewServer(http.NotFoundHandler()) + defer elsewhere.Close() + calls := 0 + home := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + http.Redirect(w, r, elsewhere.URL, http.StatusTemporaryRedirect) + })) + defer home.Close() + cl := &http.Client{CheckRedirect: redirectguard.StayOnOrigin()} + _, err := SendWithRetry(WithRetryLimit(context.Background(), 2), cl, SendOptions{Provider: "p"}, func(ctx context.Context) (*http.Request, error) { + return http.NewRequestWithContext(ctx, http.MethodPost, home.URL, nil) + }) + var left *redirectguard.OriginLeft + if !errors.As(err, &left) || left.To == "" { + t.Fatalf("err = %v, want it to name the host it was redirected to", err) + } + if calls != 1 { + t.Fatalf("a refused redirect was attempted %d times, want 1", calls) + } +} + func TestPermanentTransportErrReadsIdentity(t *testing.T) { refused := &net.OpError{Op: "dial", Net: "tcp", Err: os.NewSyscallError("connect", syscall.ECONNREFUSED)} reset := &net.OpError{Op: "read", Net: "tcp", Err: os.NewSyscallError("read", syscall.ECONNRESET)} diff --git a/internal/frontend/serve/provider_check.go b/internal/frontend/serve/provider_check.go index 0da380edd1..29de63d2b1 100644 --- a/internal/frontend/serve/provider_check.go +++ b/internal/frontend/serve/provider_check.go @@ -19,6 +19,7 @@ import ( "reasonix/internal/base/secrets" "reasonix/internal/contract/config" "reasonix/internal/contract/provider" + "reasonix/internal/safety/redirectguard" "reasonix/internal/safety/typesafe" ) @@ -41,7 +42,9 @@ type providerCheck struct { NoProxy bool `json:"noProxy,omitempty"` // Code is why the check failed, as the dotted identity the add flow's // refusals use. Params carry only the numbers its sentence needs. - Code string `json:"code,omitempty"` + Code string `json:"code,omitempty"` + // Target is the host a refused redirect pointed at, for the sentence. + Target string `json:"target,omitempty"` Params map[string]int `json:"params,omitempty"` // HTTPStatus and Detail are what the endpoint answered with, for the user // to read; neither is an input to Code. @@ -128,6 +131,10 @@ func probeFinding(err error, apiKey func() string) providerCheck { } _, code := probeReasonRefusal(probe.Reason) found := providerCheck{Code: code, HTTPStatus: probe.Status, Detail: endpointDetail(probe.Body, apiKey)} + var left *redirectguard.OriginLeft + if errors.As(err, &left) { + found.Detail, found.Target = "", left.To + } for name, value := range probe.Params { if n, ok := value.(int); ok { if found.Params == nil { @@ -295,7 +302,10 @@ func modelCheckDetail(err error, apiKey func() string) string { var auth *provider.AuthError var apiErr *provider.APIError var streamErr *provider.StreamPayloadError + var left *redirectguard.OriginLeft switch { + case errors.As(err, &left): + text = left.Error() case errors.As(err, &typeSafeErr): text = typeSafeErr.Body case errors.As(err, &auth): @@ -455,6 +465,9 @@ func classifyProviderModelCheck(err error) (status, reason string, httpStatus in if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { return "unknown", "timeout", 0 } + if errors.Is(err, redirectguard.ErrRefused) { + return "unknown", "redirect_refused", 0 + } var netErr net.Error if errors.As(err, &netErr) || provider.IsStreamInterrupted(err) { return "unknown", "network", 0 diff --git a/internal/frontend/serve/provider_check_decision.go b/internal/frontend/serve/provider_check_decision.go index 8844a48528..c73f88f2ea 100644 --- a/internal/frontend/serve/provider_check_decision.go +++ b/internal/frontend/serve/provider_check_decision.go @@ -5,6 +5,7 @@ import ( "errors" "net" "net/http" + "reasonix/internal/safety/endpointclient" "reasonix/internal/base/netclient" "reasonix/internal/contract/config" @@ -36,7 +37,7 @@ func probeDecision(ctx context.Context, cfg *config.Config, entry *config.Provid if entry.NoProxy && proxy.Mode != netclient.ModeCustom { proxy = netclient.ProxySpec{Mode: netclient.ModeOff} } - client, err := netclient.NewHTTPClient(proxy, netclient.TransportOptions{}) + client, err := endpointclient.New(proxy, netclient.TransportOptions{}) if err != nil { return errors.New("decision probe client: " + err.Error()) } diff --git a/internal/frontend/serve/provider_redirect_test.go b/internal/frontend/serve/provider_redirect_test.go new file mode 100644 index 0000000000..52405b8618 --- /dev/null +++ b/internal/frontend/serve/provider_redirect_test.go @@ -0,0 +1,70 @@ +package serve + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" +) + +// Every request that carries a provider key to a configured address must stay +// on that address: a redirect elsewhere is refused before the key is resent. +func TestProviderProbesRefuseACrossOriginRedirect(t *testing.T) { + for _, tc := range []struct { + name, path, kind string + status int + }{ + {"single-model check", "/providers/check/model", "openai", http.StatusTemporaryRedirect}, + {"decision probe", "/providers/check/model", "typesafe", http.StatusPermanentRedirect}, + {"endpoint probe", "/providers/probe", "openai", http.StatusFound}, + } { + t.Run(tc.name, func(t *testing.T) { + var reached atomic.Int32 + elsewhere := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + reached.Add(1) + })) + defer elsewhere.Close() + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, elsewhere.URL+r.URL.Path, tc.status) + })) + defer upstream.Close() + + s := newProviderEditServer(t) + s.AllowProviderEdit() + srv := httptest.NewServer(operatorHandler(s)) + defer srv.Close() + body := fmt.Sprintf(`{"name":"not-saved","model":"m","baseUrl":%q,"apiKey":"secret-key","kind":%q}`, upstream.URL+"/v1", tc.kind) + if tc.path == "/providers/probe" { + body = fmt.Sprintf(`{"baseUrl":%q,"apiKey":"secret-key"}`, upstream.URL+"/v1") + } + resp := postProvider(t, srv.URL, tc.path, body) + defer resp.Body.Close() + var got struct { + Code string `json:"code"` + Reason string `json:"reason"` + Detail string `json:"detail"` + Msg string `json:"error"` + } + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { + t.Fatal(err) + } + host := strings.TrimPrefix(elsewhere.URL, "http://") + switch tc.path { + case "/providers/probe": + if got.Code != "provider.probe.redirect_refused" || !strings.Contains(got.Msg, host) { + t.Fatalf("probe refusal = %+v, want redirect_refused naming %s", got, host) + } + default: + if got.Reason != "redirect_refused" || !strings.Contains(got.Detail, host) { + t.Fatalf("model check = %+v, want redirect_refused naming %s", got, host) + } + } + if n := reached.Load(); n != 0 { + t.Fatalf("the redirect target received %d request(s); the key left the configured address", n) + } + }) + } +} diff --git a/internal/frontend/serve/providers.go b/internal/frontend/serve/providers.go index 8fc51c2236..06efa9a50a 100644 --- a/internal/frontend/serve/providers.go +++ b/internal/frontend/serve/providers.go @@ -15,6 +15,8 @@ import ( "reasonix/internal/base/netclient" "reasonix/internal/contract/config" "reasonix/internal/contract/provider" + "reasonix/internal/safety/endpointclient" + "reasonix/internal/safety/redirectguard" ) // hostGrants are the surfaces a host opens on behalf of its one local client. @@ -493,8 +495,8 @@ func probeClients() (proxied, direct *http.Client) { if cfg, err := config.Load(); err == nil && cfg != nil { spec = cfg.NetworkProxySpec() } - proxied, _ = netclient.NewHTTPClient(spec, netclient.TransportOptions{}) - direct, _ = netclient.NewHTTPClient(netclient.ProxySpec{Mode: netclient.ModeOff}, netclient.TransportOptions{}) + proxied, _ = endpointclient.New(spec, netclient.TransportOptions{}) + direct, _ = endpointclient.New(netclient.ProxySpec{Mode: netclient.ModeOff}, netclient.TransportOptions{}) return proxied, direct } @@ -523,6 +525,7 @@ const ( codeProbeNoChatModels = "provider.probe.no_chat_models" codeProbeUpstreamError = "provider.probe.upstream_error" codeProbeTimeout = "provider.probe.timeout" + codeProbeRedirectRefused = "provider.probe.redirect_refused" codeProbeUnreachable = "provider.probe.unreachable" codeProbeNotCompatible = "provider.probe.not_compatible" codeProbeFailed = "provider.probe.failed" @@ -555,6 +558,8 @@ func probeReasonRefusal(reason catalog.ProbeReason) (status int, code string) { return http.StatusGatewayTimeout, codeProbeTimeout case catalog.ProbeUnreachable: return http.StatusBadGateway, codeProbeUnreachable + case catalog.ProbeRedirectRefused: + return http.StatusBadGateway, codeProbeRedirectRefused default: return http.StatusBadGateway, codeProbeNotCompatible } @@ -574,5 +579,10 @@ func writeProbeFailure(w http.ResponseWriter, err error, apiKey string) { if detail := endpointDetail(probe.Body, func() string { return apiKey }); detail != "" { message += ": " + detail } + var left *redirectguard.OriginLeft + if errors.As(err, &left) { + message = left.Error() + probe.Params = map[string]any{"from": left.From, "target": left.To} + } refuse(w, status, code, message, probe.Params) } diff --git a/internal/model/anthropic/anthropic.go b/internal/model/anthropic/anthropic.go index ad155c9b48..56c6226203 100644 --- a/internal/model/anthropic/anthropic.go +++ b/internal/model/anthropic/anthropic.go @@ -36,6 +36,7 @@ import ( "reasonix/internal/base/netclient" "reasonix/internal/contract/provider" "reasonix/internal/model/openai" + "reasonix/internal/safety/endpointclient" ) // defaultStreamIdleTimeout caps how long a started SSE stream may go silent before @@ -154,7 +155,7 @@ func New(cfg provider.Config) (provider.Provider, error) { func newHTTPClient(cfg provider.Config) (*http.Client, error) { spec, _ := cfg.Extra["proxy_spec"].(netclient.ProxySpec) - return netclient.NewHTTPClient(spec, netclient.TransportOptions{ResponseHeaderTimeout: provider.IdleTimeoutFromExtra(cfg.Extra)}) + return endpointclient.New(spec, netclient.TransportOptions{ResponseHeaderTimeout: provider.IdleTimeoutFromExtra(cfg.Extra)}) } type client struct { diff --git a/internal/model/billing/balance.go b/internal/model/billing/balance.go index abab10b760..647b845d5c 100644 --- a/internal/model/billing/balance.go +++ b/internal/model/billing/balance.go @@ -17,6 +17,8 @@ import ( "strconv" "strings" "time" + + "reasonix/internal/safety/redirectguard" ) // A wallet read fails in ways the reader must tell apart: a rejected credential @@ -101,7 +103,7 @@ func positive(amount string) bool { // httpClient bounds the balance query so a slow endpoint can't hang the status // line; the per-call ctx still cancels it on shutdown. -var httpClient = &http.Client{Timeout: 12 * time.Second} +var httpClient = &http.Client{Timeout: 12 * time.Second, CheckRedirect: redirectguard.StayOnOrigin()} // Fetch queries url (a DeepSeek-style balance endpoint) with a Bearer apiKey and // returns the normalized balance. An empty url yields (nil, nil) — "not diff --git a/internal/model/billing/balance_redirect_test.go b/internal/model/billing/balance_redirect_test.go new file mode 100644 index 0000000000..7c169db692 --- /dev/null +++ b/internal/model/billing/balance_redirect_test.go @@ -0,0 +1,25 @@ +package billing + +import ( + "context" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +func TestFetchDoesNotFollowARedirectWithTheKey(t *testing.T) { + var reached atomic.Int32 + elsewhere := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached.Add(1) })) + defer elsewhere.Close() + home := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, elsewhere.URL, http.StatusTemporaryRedirect) + })) + defer home.Close() + if _, err := Fetch(context.Background(), home.URL, "secret"); err == nil { + t.Fatal("a cross-origin redirect was followed") + } + if n := reached.Load(); n != 0 { + t.Fatalf("redirect target received %d request(s)", n) + } +} diff --git a/internal/model/catalog/probe_failure.go b/internal/model/catalog/probe_failure.go index b98c6995d9..4f8c4383ed 100644 --- a/internal/model/catalog/probe_failure.go +++ b/internal/model/catalog/probe_failure.go @@ -9,6 +9,7 @@ import ( "net/http" "reasonix/internal/model/openai" + "reasonix/internal/safety/redirectguard" ) // ProbeReason names why probing failed. Each one sends the user to a different @@ -27,6 +28,7 @@ const ( ProbeTimeout ProbeReason = "timeout" ProbeUnreachable ProbeReason = "unreachable" ProbeNotCompatible ProbeReason = "not_compatible" + ProbeRedirectRefused ProbeReason = "redirect_refused" ) // ProbeError carries that identity out. Params hold the facts a sentence needs @@ -56,6 +58,7 @@ func (e *ProbeError) Unwrap() error { return e.err } // listing that answered with no chat models settles the question, a 401 says // more than a 404, and "not compatible" is what is left when nothing spoke. var probeReasonRank = map[ProbeReason]int{ + ProbeRedirectRefused: -1, ProbeNoChatModels: 0, ProbePaymentRequired: 1, ProbeUnauthorized: 2, @@ -74,6 +77,9 @@ func classifyProbe(err error) *ProbeError { if errors.As(err, &known) { return known } + if errors.Is(err, redirectguard.ErrRefused) { + return &ProbeError{Reason: ProbeRedirectRefused, err: err} + } status, ok := openai.ModelFetchStatus(err) if !ok { var netErr net.Error diff --git a/internal/model/openai/fetch_models.go b/internal/model/openai/fetch_models.go index 9903eead4f..0ec830fd7d 100644 --- a/internal/model/openai/fetch_models.go +++ b/internal/model/openai/fetch_models.go @@ -12,6 +12,7 @@ import ( "time" "reasonix/internal/contract/provider" + "reasonix/internal/safety/redirectguard" ) type modelFetchStatusError struct { @@ -107,7 +108,7 @@ type ListedModel struct { func FetchModelListing(ctx context.Context, baseURL, apiKey string, opts FetchModelsOptions) ([]ListedModel, error) { cli := opts.Client if cli == nil { - cli = &http.Client{Timeout: 10 * time.Second} + cli = &http.Client{Timeout: 10 * time.Second, CheckRedirect: redirectguard.StayOnOrigin()} } url := strings.TrimRight(baseURL, "/") if !strings.HasSuffix(url, "/models") { diff --git a/internal/model/openai/fetch_models_redirect_test.go b/internal/model/openai/fetch_models_redirect_test.go new file mode 100644 index 0000000000..6848455efb --- /dev/null +++ b/internal/model/openai/fetch_models_redirect_test.go @@ -0,0 +1,25 @@ +package openai + +import ( + "context" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +func TestFetchModelListingWithoutAClientDoesNotFollowARedirectWithTheKey(t *testing.T) { + var reached atomic.Int32 + elsewhere := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached.Add(1) })) + defer elsewhere.Close() + home := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, elsewhere.URL, http.StatusTemporaryRedirect) + })) + defer home.Close() + if _, err := FetchModelListing(context.Background(), home.URL+"/v1", "secret", FetchModelsOptions{}); err == nil { + t.Fatal("a cross-origin redirect was followed") + } + if n := reached.Load(); n != 0 { + t.Fatalf("redirect target received %d request(s)", n) + } +} diff --git a/internal/model/openai/openai.go b/internal/model/openai/openai.go index b9ef7e8473..2f804ef881 100644 --- a/internal/model/openai/openai.go +++ b/internal/model/openai/openai.go @@ -37,6 +37,7 @@ import ( "reasonix/internal/base/netclient" "reasonix/internal/contract/provider" + "reasonix/internal/safety/endpointclient" ) // defaultStreamIdleTimeout caps how long a started SSE stream may go without any @@ -264,7 +265,7 @@ func New(cfg provider.Config) (provider.Provider, error) { func newHTTPClient(cfg provider.Config) (*http.Client, error) { spec, _ := cfg.Extra["proxy_spec"].(netclient.ProxySpec) - return netclient.NewHTTPClient(spec, netclient.TransportOptions{ + return endpointclient.New(spec, netclient.TransportOptions{ DialTimeout: 30 * time.Second, KeepAlive: 30 * time.Second, TLSHandshakeTimeout: 15 * time.Second, diff --git a/internal/model/responses/responses.go b/internal/model/responses/responses.go index a2b662a3d8..cecffe7ebd 100644 --- a/internal/model/responses/responses.go +++ b/internal/model/responses/responses.go @@ -20,6 +20,8 @@ import ( "reasonix/internal/base/netclient" "reasonix/internal/contract/provider" provideropenai "reasonix/internal/model/openai" + "reasonix/internal/safety/endpointclient" + "reasonix/internal/safety/redirectguard" ) const ( @@ -164,8 +166,8 @@ func New(cfg Config) provider.Provider { if vendor == "deepseek" { vision = vision && provideropenai.DeepSeekTakesImages(cfg.Model) } - httpClient := &http.Client{Timeout: 300 * time.Second} - if built, err := netclient.NewHTTPClient(cfg.Proxy, netclient.TransportOptions{ + httpClient := &http.Client{Timeout: 300 * time.Second, CheckRedirect: redirectguard.StayOnOrigin()} + if built, err := endpointclient.New(cfg.Proxy, netclient.TransportOptions{ DialTimeout: 30 * time.Second, KeepAlive: 30 * time.Second, TLSHandshakeTimeout: 15 * time.Second, ResponseHeaderTimeout: provider.IdleTimeoutFromExtra(cfg.Extra), }); err == nil { diff --git a/internal/platform/repair/diagnose.go b/internal/platform/repair/diagnose.go index b8ece07a00..49d27f7ee7 100644 --- a/internal/platform/repair/diagnose.go +++ b/internal/platform/repair/diagnose.go @@ -3,6 +3,7 @@ package repair import ( "context" "encoding/json" + "errors" "fmt" "maps" "net/http" @@ -18,6 +19,8 @@ import ( "reasonix/internal/base/netclient" "reasonix/internal/contract/config" + "reasonix/internal/safety/endpointclient" + "reasonix/internal/safety/redirectguard" ) type DiagnosticFinding struct { @@ -295,7 +298,7 @@ func checkDerivedJSON(report *DiagnosticReport) { } func probeProviderNetwork(ctx context.Context, report *DiagnosticReport, cfg *config.Config, timeout time.Duration) { - client, err := netclient.NewHTTPClient(cfg.NetworkProxySpec(), netclient.TransportOptions{DialTimeout: timeout, TLSHandshakeTimeout: timeout, ResponseHeaderTimeout: timeout}) + client, err := endpointclient.New(cfg.NetworkProxySpec(), netclient.TransportOptions{DialTimeout: timeout, TLSHandshakeTimeout: timeout, ResponseHeaderTimeout: timeout}) if err != nil { report.add("error", "network.client_failed", "network", "Cannot build network client: "+err.Error(), "Correct proxy settings.") return @@ -324,7 +327,9 @@ func probeProviderNetwork(ctx context.Context, report *DiagnosticReport, cfg *co } } resp, callErr := client.Do(req) - if callErr != nil { + if errors.Is(callErr, redirectguard.ErrRefused) { + report.add("warning", "network.redirect_refused", "provider:"+entry.Name, "Provider endpoint redirected to another host and was not followed.", "If you trust the new host, set the provider's base_url to it.") + } else if callErr != nil { report.add("warning", "network.unreachable", "provider:"+entry.Name, "Provider endpoint could not be reached: "+redactNetworkError(callErr), "Check DNS, proxy, firewall, and provider availability.") } else { _ = resp.Body.Close() diff --git a/internal/platform/repair/diagnose_redirect_test.go b/internal/platform/repair/diagnose_redirect_test.go new file mode 100644 index 0000000000..37edd25c4e --- /dev/null +++ b/internal/platform/repair/diagnose_redirect_test.go @@ -0,0 +1,34 @@ +package repair + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "reasonix/internal/contract/config" +) + +func TestProbeProviderNetworkDoesNotFollowARedirectWithTheKey(t *testing.T) { + var reached atomic.Int32 + elsewhere := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached.Add(1) })) + defer elsewhere.Close() + home := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, elsewhere.URL, http.StatusTemporaryRedirect) + })) + defer home.Close() + t.Setenv("DOCTOR_PROBE_KEY", "secret") + cfg := &config.Config{Providers: []config.ProviderEntry{{Name: "p", Kind: "openai", BaseURL: home.URL + "/v1", APIKeyEnv: "DOCTOR_PROBE_KEY", Headers: map[string]string{"x-extra": "1"}}}} + var report DiagnosticReport + probeProviderNetwork(t.Context(), &report, cfg, 5*time.Second) + if n := reached.Load(); n != 0 { + t.Fatalf("redirect target received %d request(s)", n) + } + for _, f := range report.Findings { + if f.Code == "network.redirect_refused" { + return + } + } + t.Fatalf("findings = %+v, want network.redirect_refused", report.Findings) +} diff --git a/internal/safety/endpointclient/endpointclient.go b/internal/safety/endpointclient/endpointclient.go new file mode 100644 index 0000000000..8a5685a5c7 --- /dev/null +++ b/internal/safety/endpointclient/endpointclient.go @@ -0,0 +1,21 @@ +// Package endpointclient builds the HTTP client for requests that carry a +// credential to an address the user configured. A redirect that leaves that +// origin is refused before the request, and the credential with it, is resent. +package endpointclient + +import ( + "net/http" + + "reasonix/internal/base/netclient" + "reasonix/internal/safety/redirectguard" +) + +// New is netclient.NewHTTPClient with the redirect policy of redirectguard.StayOnOrigin. +func New(spec netclient.ProxySpec, opts netclient.TransportOptions) (*http.Client, error) { + c, err := netclient.NewHTTPClient(spec, opts) + if err != nil { + return nil, err + } + c.CheckRedirect = redirectguard.StayOnOrigin() + return c, nil +} diff --git a/internal/safety/endpointclient/endpointclient_test.go b/internal/safety/endpointclient/endpointclient_test.go new file mode 100644 index 0000000000..6e4dd994a0 --- /dev/null +++ b/internal/safety/endpointclient/endpointclient_test.go @@ -0,0 +1,45 @@ +package endpointclient + +import ( + "errors" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "reasonix/internal/base/netclient" + "reasonix/internal/safety/redirectguard" +) + +func TestNewRefusesARedirectOffTheConfiguredOrigin(t *testing.T) { + var reached atomic.Int32 + elsewhere := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached.Add(1) })) + defer elsewhere.Close() + home := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v2": + case "/moved": + http.Redirect(w, r, "/v2", http.StatusTemporaryRedirect) + default: + http.Redirect(w, r, elsewhere.URL, http.StatusTemporaryRedirect) + } + })) + defer home.Close() + client, err := New(netclient.ProxySpec{Mode: netclient.ModeOff}, netclient.TransportOptions{}) + if err != nil { + t.Fatal(err) + } + + if _, err := client.Post(home.URL, "application/json", strings.NewReader("{}")); !errors.Is(err, redirectguard.ErrRefused) { + t.Fatalf("cross-origin redirect error = %v, want redirectguard.ErrRefused", err) + } + if n := reached.Load(); n != 0 { + t.Fatalf("redirect target received %d request(s)", n) + } + resp, err := client.Post(home.URL+"/moved", "application/json", strings.NewReader("{}")) + if err != nil { + t.Fatalf("same-origin redirect refused: %v", err) + } + resp.Body.Close() +} diff --git a/internal/safety/redirectguard/redirectguard.go b/internal/safety/redirectguard/redirectguard.go index d00cbdd1b0..3e88f90582 100644 --- a/internal/safety/redirectguard/redirectguard.go +++ b/internal/safety/redirectguard/redirectguard.go @@ -7,16 +7,32 @@ package redirectguard import ( - "errors" "fmt" "net/http" + "net/url" "strings" ) // ErrRefused identifies a redirect that was not followed, so a caller can tell // one from a transport failure. The message names the rule that stopped it and // is for whoever reads the log. -var ErrRefused = errors.New("redirect refused") +var ErrRefused error = refused{} + +// refused is a failure no retry can fix: the same request goes to the same place. +type refused struct{} + +func (refused) Error() string { return "redirect refused" } +func (refused) Permanent() bool { return true } + +// OriginLeft is the refusal of a redirect to another host. From and To are +// host[:port] only, so the message is safe to show and carries no path or query. +type OriginLeft struct{ From, To string } + +func (e *OriginLeft) Error() string { + return fmt.Sprintf("redirect refused: %s redirected to %s; if you trust %s, set the address to %s", e.From, e.To, e.To, e.To) +} + +func (e *OriginLeft) Unwrap() error { return ErrRefused } // maxHops is Go's own default, restated because a CheckRedirect replaces that // default outright: without a limit here a chain never stops. @@ -52,6 +68,50 @@ func Follow(hosts ...string) func(*http.Request, []*http.Request) error { } } +// StayOnOrigin returns a CheckRedirect for a request to an address the user +// chose, which may be HTTP or carry a port. A redirect is followed only on the +// host the chain started on (same port, or the default-port HTTP to HTTPS +// upgrade), never downgrades HTTPS and carries no credentials. +func StayOnOrigin() func(*http.Request, []*http.Request) error { + return func(req *http.Request, via []*http.Request) error { + if len(via) >= maxHops { + return fmt.Errorf("%w: stopped after %d hops", ErrRefused, maxHops) + } + if req == nil || req.URL == nil || len(via) == 0 || via[0].URL == nil { + return fmt.Errorf("%w: no origin to stay on", ErrRefused) + } + origin, target := via[0].URL, req.URL + if target.User != nil { + return fmt.Errorf("%w: carries credentials: %q", ErrRefused, target.String()) + } + if strings.EqualFold(origin.Scheme, "https") && !strings.EqualFold(target.Scheme, "https") { + return fmt.Errorf("%w: downgrades HTTPS: %q", ErrRefused, target.String()) + } + if !sameOrigin(origin, target) { + return &OriginLeft{From: origin.Host, To: target.Host} + } + return nil + } +} + +func sameOrigin(origin, target *url.URL) bool { + if !strings.EqualFold(strings.TrimSuffix(origin.Hostname(), "."), strings.TrimSuffix(target.Hostname(), ".")) { + return false + } + from, to := effectivePort(origin), effectivePort(target) + return from == to || (from == "80" && to == "443" && !strings.EqualFold(origin.Scheme, "https")) +} + +func effectivePort(u *url.URL) string { + if p := u.Port(); p != "" { + return p + } + if strings.EqualFold(u.Scheme, "https") { + return "443" + } + return "80" +} + // permitted compares the way DNS does: case-insensitively, and with the root // label a resolver accepts but a string comparison would not. func permitted(host string, hosts []string) bool { diff --git a/internal/safety/redirectguard/redirectguard_test.go b/internal/safety/redirectguard/redirectguard_test.go index 1f4b8f8e0c..d35e8a904e 100644 --- a/internal/safety/redirectguard/redirectguard_test.go +++ b/internal/safety/redirectguard/redirectguard_test.go @@ -3,6 +3,7 @@ package redirectguard import ( "errors" "net/http" + "strings" "testing" ) @@ -91,3 +92,77 @@ func TestFollowWithNoHostsTrustsNothing(t *testing.T) { t.Fatal("a guard with no trusted hosts followed a redirect anyway") } } + +func TestStayOnOriginFollowsOnlyTheHostItStartedOn(t *testing.T) { + for _, tc := range []struct { + name, from, to string + hops int + want bool + }{ + {"same host and port", "http://localhost:8080/v1", "http://localhost:8080/v2", 1, true}, + {"path-only move on https", "https://api.example.com/v1", "https://api.example.com/v2", 1, true}, + {"http to https upgrade on default ports", "http://relay.example.com/v1", "https://relay.example.com/v1", 1, true}, + {"host case and root label", "https://API.example.com/v1", "https://api.example.com./v1", 1, true}, + {"another port on the same host", "http://127.0.0.1:1/v1", "http://127.0.0.1:2/v1", 1, false}, + {"another host", "https://api.example.com/v1", "https://evil.test/v1", 1, false}, + {"a sibling subdomain", "https://api.example.com/v1", "https://cdn.example.com/v1", 1, false}, + {"https downgraded to http", "https://api.example.com/v1", "http://api.example.com/v1", 1, false}, + {"credentials in the target", "https://api.example.com/v1", "https://u:p@api.example.com/v1", 1, false}, + {"too many hops", "https://api.example.com/v1", "https://api.example.com/v2", 10, false}, + } { + t.Run(tc.name, func(t *testing.T) { + first, err := http.NewRequest(http.MethodPost, tc.from, nil) + if err != nil { + t.Fatal(err) + } + next, err := http.NewRequest(http.MethodPost, tc.to, nil) + if err != nil { + t.Fatal(err) + } + via := make([]*http.Request, tc.hops) + for i := range via { + via[i] = first + } + err = StayOnOrigin()(next, via) + if (err == nil) != tc.want { + t.Fatalf("StayOnOrigin %s -> %s = %v, want follow=%v", tc.from, tc.to, err, tc.want) + } + if err != nil && !errors.Is(err, ErrRefused) { + t.Fatalf("refusal %v does not carry ErrRefused", err) + } + }) + } +} + +func TestStayOnOriginNamesTheHostItWasSentTo(t *testing.T) { + first, _ := http.NewRequest(http.MethodPost, "https://example.com/v1", nil) + next, _ := http.NewRequest(http.MethodPost, "https://www.example.com/v1?token=x", nil) + err := StayOnOrigin()(next, []*http.Request{first}) + var left *OriginLeft + if !errors.As(err, &left) || left.From != "example.com" || left.To != "www.example.com" { + t.Fatalf("err = %#v, want OriginLeft example.com -> www.example.com", err) + } + if !errors.Is(err, ErrRefused) || strings.Contains(err.Error(), "token") { + t.Fatalf("err = %v: must be ErrRefused and carry no query", err) + } +} + +func TestRefusalsAreRecognisedAndPermanent(t *testing.T) { + var p interface{ Permanent() bool } + if !errors.As(ErrRefused, &p) || !p.Permanent() { + t.Fatal("ErrRefused must report itself permanent") + } + if err := Follow(releaseHosts...)(&http.Request{}, nil); !errors.Is(err, ErrRefused) { + t.Fatalf("a request with no URL = %v, want ErrRefused", err) + } + empty, _ := http.NewRequest(http.MethodGet, "https:///x", nil) + if err := Follow(releaseHosts...)(empty, nil); !errors.Is(err, ErrRefused) { + t.Fatalf("a URL with no hostname = %v, want ErrRefused", err) + } + if err := StayOnOrigin()(empty, nil); !errors.Is(err, ErrRefused) { + t.Fatalf("no earlier request to stay on = %v, want ErrRefused", err) + } + if permitted("", releaseHosts) || permitted(" . ", releaseHosts) { + t.Fatal("an empty host was permitted") + } +} diff --git a/internal/safety/typesafe/client.go b/internal/safety/typesafe/client.go index a34127e1b0..d7eb075b95 100644 --- a/internal/safety/typesafe/client.go +++ b/internal/safety/typesafe/client.go @@ -11,6 +11,8 @@ import ( "net/http" "net/url" "strings" + + "reasonix/internal/safety/redirectguard" ) const DefaultBaseURL = "https://api.typesafe.ai" @@ -90,7 +92,7 @@ func (c Client) Evaluate(ctx context.Context, request Request) (Response, error) httpRequest.Header.Set("Content-Type", "application/json") client := c.HTTP if client == nil { - client = http.DefaultClient + client = &http.Client{CheckRedirect: redirectguard.StayOnOrigin()} } httpResponse, err := client.Do(httpRequest) if err != nil { diff --git a/internal/safety/typesafe/client_redirect_test.go b/internal/safety/typesafe/client_redirect_test.go new file mode 100644 index 0000000000..a3594961a7 --- /dev/null +++ b/internal/safety/typesafe/client_redirect_test.go @@ -0,0 +1,26 @@ +package typesafe + +import ( + "context" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +func TestEvaluateWithoutAClientDoesNotFollowARedirectWithTheKey(t *testing.T) { + var reached atomic.Int32 + elsewhere := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached.Add(1) })) + defer elsewhere.Close() + home := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, elsewhere.URL, http.StatusTemporaryRedirect) + })) + defer home.Close() + _, err := (Client{BaseURL: home.URL, APIKey: func() string { return "secret" }}).Evaluate(context.Background(), Request{State: "s", Model: "m"}) + if err == nil { + t.Fatal("a cross-origin redirect was followed") + } + if n := reached.Load(); n != 0 { + t.Fatalf("redirect target received %d request(s)", n) + } +} diff --git a/internal/session/control/network.go b/internal/session/control/network.go index 56132b2fe0..58a965d414 100644 --- a/internal/session/control/network.go +++ b/internal/session/control/network.go @@ -9,6 +9,7 @@ import ( "reasonix/internal/base/netclient" "reasonix/internal/contract/config" + "reasonix/internal/safety/endpointclient" ) // NetworkSettings is the proxy configuration as an editor needs it: the mode, @@ -123,7 +124,7 @@ func (c *Controller) probeProviderAuth(ctx context.Context, cfg *config.Config, probe.DurationMs = time.Since(started).Milliseconds() return probe } - client, err := netclient.NewHTTPClient(spec, netclient.TransportOptions{DialTimeout: 8 * time.Second}) + client, err := endpointclient.New(spec, netclient.TransportOptions{DialTimeout: 8 * time.Second}) if err != nil { probe.Detail = err.Error() return probe diff --git a/tools/covergate/baseline.json b/tools/covergate/baseline.json index 0687ab8175..2f2d4424c1 100644 --- a/tools/covergate/baseline.json +++ b/tools/covergate/baseline.json @@ -11,6 +11,7 @@ "internal/ext/pluginpkg/**": 82.7, "internal/model/providerbroker/**": 83.1, "internal/safety/redirectguard/**": 92.6, + "internal/safety/endpointclient/**": 80, "internal/safety/egress/**": 77.1, "internal/safety/sandbox/**": 78.1, "internal/base/shellparse/**": 64.5,