refactor(net): one redirect guard, three trust decisions

The same judgement existed three times: validateCLIUpgradeRedirect,
validateOfficialRedirect, and the update package's TrustedRedirect. The
first and third are line-for-line identical apart from an error prefix and
the name of the host predicate; the second is the same rules folded into
one condition. The third had never been wired to a client at all, which is
the shape this kind of duplication fails in -- a copy nobody is looking at
stops being maintained before anyone notices it stopped being called.

internal/redirectguard holds the mechanism: HTTPS only, no credentials, no
port, a hop limit, and a host list the caller names. ErrRefused gives the
refusal an identity so a caller tells one from a transport failure through
errors.Is rather than by reading the sentence.

The host lists stay apart on purpose. Which hosts a download trusts is
that path's own security decision, and that the CLI upgrade and the remote
CLI provisioner both name only GitHub is a fact about where we publish,
not one about them. Merged, a change to either would silently move the
other. Mechanism shared, policy separate.

The layering rule is what had made this impossible: it refused a leaf
importing anything under reasonix/, including another leaf, while its
stated intent is that a utility drags no dependency graph along. A closed
layer reaching inside itself drags nothing, and Go already refuses a cycle.
So the rule now says what it meant -- a leaf may import a leaf -- which is
a sharpening rather than a loosening, and releaseasset can share the guard
instead of keeping the copy the old rule forced it to keep.

redirectguard joins the sensitive list beside netclient: it decides which
host a download may land on, and a new security boundary outside that
governance is one nobody reviews. One declaration, both effects -- the
review gate and a coverage floor, recorded at 92.6%.

REASONIX.md

@@ -11,7 +11,9 @@ agent. It is the Reasonix analog of Claude Code's CLAUDE.md.
 - One transport-agnostic `control.Controller` sits behind every frontend (chat
   TUI, HTTP/SSE serve, Wails desktop). Add behavior to the controller, not a
   frontend, so all three inherit it.
-- Layering (enforced): utility packages import nothing under `reasonix/`; only
+- Layering (enforced): utility packages import nothing under `reasonix/` except
+  one another — that layer is closed, so a leaf reaching a leaf drags no graph
+  along, while refusing it leaves one judgement copied into several of them; only
   the frontends `cli`, `serve`, `acp`, `bot`, `botruntime`, `boot` and the hosts
   `cmd/`, `desktop/` may import `control`; nothing below a frontend may import
   one. The declared sets live in `tools/repolint/layers.go`.
@@ -215,6 +217,7 @@ out of how a path is spelled, which cannot tell `internal/auth` from
 - sensitive: internal/plugin/**
 - sensitive: internal/pluginpkg/**
 - sensitive: internal/netclient/**
+- sensitive: internal/redirectguard/**
 
 One declaration, two effects: the same list is also the coverage gate's subject.
 `make coverage-gate` holds each path to the coverage it already has, because

internal/cli/upgrade.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/spf13/pflag"
 	"golang.org/x/mod/semver"
+	"reasonix/internal/redirectguard"
 )
 
 const (
@@ -228,7 +229,7 @@ func upgradeCommand(args []string, version string) int {
 		fmt.Fprintf(os.Stderr, "%s %v\n", i18n.M.ErrorPrefix, err)
 		return 1
 	}
-	c.CheckRedirect = validateCLIUpgradeRedirect
+	c.CheckRedirect = redirectguard.Follow(cliUpgradeHosts...)
 
 	// 3. Fetch latest release from GitHub API.
 	fmt.Println(i18n.M.UpgradeChecking)
@@ -388,32 +389,10 @@ func isExpectedCLIAssetURL(raw, tag, name string) bool {
 		parsed.Fragment == ""
 }
 
-func isTrustedCLIUpgradeRedirectHost(host string) bool {
-	host = strings.ToLower(strings.TrimSuffix(strings.TrimSpace(host), "."))
-	return host == "github.com" || strings.HasSuffix(host, ".githubusercontent.com")
-}
-
-func validateCLIUpgradeRedirect(req *http.Request, via []*http.Request) error {
-	if len(via) >= 10 {
-		return errors.New("upgrade: stopped after 10 redirects")
-	}
-	if req == nil || req.URL == nil {
-		return errors.New("upgrade: redirect has no target URL")
-	}
-	if !strings.EqualFold(req.URL.Scheme, "https") {
-		return fmt.Errorf("upgrade: refusing redirect to non-HTTPS URL %q", req.URL.String())
-	}
-	if req.URL.Hostname() == "" {
-		return fmt.Errorf("upgrade: refusing redirect without a hostname %q", req.URL.String())
-	}
-	if req.URL.User != nil {
-		return fmt.Errorf("upgrade: refusing redirect with userinfo %q", req.URL.String())
-	}
-	if req.URL.Port() != "" || !isTrustedCLIUpgradeRedirectHost(req.URL.Hostname()) {
-		return fmt.Errorf("upgrade: refusing redirect to untrusted host %q", req.URL.Host)
-	}
-	return nil
-}
+// cliUpgradeHosts is where a published CLI release is served from. It is this
+// command's own trust decision; see releaseasset.officialHosts for why the two
+// identical lists are not one.
+var cliUpgradeHosts = []string{"github.com", ".githubusercontent.com"}
 
 func validCLIAssetSize(size int64) bool {
 	return size > 0 && size <= maxCLIReleaseAssetSize

internal/cli/upgrade_test.go

@@ -17,6 +17,7 @@ import (
 	"testing"
 
 	"reasonix/internal/config"
+	"reasonix/internal/redirectguard"
 	"reasonix/internal/testenv"
 )
 
@@ -330,7 +331,11 @@ func TestFindCLIPlatformAssetRequiresExactArchiveName(t *testing.T) {
 	}
 }
 
-func TestValidateCLIUpgradeRedirect(t *testing.T) {
+// The mechanism is held by internal/redirectguard; what this holds is the
+// decision made here -- that these are the hosts a published CLI comes from --
+// by driving the guard exactly as upgrade configures it.
+func TestCLIUpgradeTrustsOnlyItsReleaseHosts(t *testing.T) {
+	follow := redirectguard.Follow(cliUpgradeHosts...)
 	tests := []struct {
 		name      string
 		target    string
@@ -351,9 +356,9 @@ func TestValidateCLIUpgradeRedirect(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			err = validateCLIUpgradeRedirect(req, nil)
+			err = follow(req, nil)
 			if (err != nil) != tt.wantError {
-				t.Fatalf("validateCLIUpgradeRedirect(%q) error = %v, wantError=%v", tt.target, err, tt.wantError)
+				t.Fatalf("follow(%q) error = %v, wantError=%v", tt.target, err, tt.wantError)
 			}
 		})
 	}
@@ -362,8 +367,8 @@ func TestValidateCLIUpgradeRedirect(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		if err := validateCLIUpgradeRedirect(req, make([]*http.Request, 10)); err == nil {
-			t.Fatal("validateCLIUpgradeRedirect accepted more than 10 redirects")
+		if err := follow(req, make([]*http.Request, 10)); err == nil {
+			t.Fatal("the upgrade guard accepted more than 10 redirects")
 		}
 	})
 }

internal/redirectguard/redirectguard.go

@@ -0,0 +1,75 @@
+// Package redirectguard decides which redirects a download may follow.
+//
+// A client that authenticates and then follows a redirect anywhere is how an
+// artifact arrives from a host nobody vouched for. Three downloads needed that
+// judgement and grew three copies of it, one of which was never wired to the
+// client it was written for; this is the one they share.
+package redirectguard
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"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")
+
+// maxHops is Go's own default, restated because a CheckRedirect replaces that
+// default outright: without a limit here a chain never stops.
+const maxHops = 10
+
+// Follow returns a CheckRedirect that follows a redirect only while it stays
+// HTTPS, carries no credentials and no port, and lands on one of hosts. A host
+// is an exact name, or a ".suffix" matching any name beneath it.
+func Follow(hosts ...string) 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 {
+			return fmt.Errorf("%w: no target URL", ErrRefused)
+		}
+		target := req.URL
+		if !strings.EqualFold(target.Scheme, "https") {
+			return fmt.Errorf("%w: not HTTPS: %q", ErrRefused, target.String())
+		}
+		if target.User != nil {
+			return fmt.Errorf("%w: carries credentials: %q", ErrRefused, target.String())
+		}
+		if target.Hostname() == "" {
+			return fmt.Errorf("%w: no hostname: %q", ErrRefused, target.String())
+		}
+		// A port is refused rather than matched: release hosts answer on 443,
+		// and an allowed name on another port is a different endpoint.
+		if target.Port() != "" || !permitted(target.Hostname(), hosts) {
+			return fmt.Errorf("%w: untrusted host %q", ErrRefused, target.Host)
+		}
+		return nil
+	}
+}
+
+// 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 {
+	host = strings.ToLower(strings.TrimSuffix(strings.TrimSpace(host), "."))
+	if host == "" {
+		return false
+	}
+	for _, allowed := range hosts {
+		allowed = strings.ToLower(allowed)
+		if strings.HasPrefix(allowed, ".") {
+			if strings.HasSuffix(host, allowed) {
+				return true
+			}
+			continue
+		}
+		if host == allowed {
+			return true
+		}
+	}
+	return false
+}

internal/redirectguard/redirectguard_test.go

@@ -0,0 +1,93 @@
+package redirectguard
+
+import (
+	"errors"
+	"net/http"
+	"testing"
+)
+
+// The hosts a Studio release is served from, used as the subject throughout.
+var releaseHosts = []string{"reasonix.io", ".reasonix.io", "github.com", ".githubusercontent.com"}
+
+func hop(t *testing.T, raw string, hops int) error {
+	t.Helper()
+	req, err := http.NewRequest(http.MethodGet, raw, nil)
+	if err != nil {
+		t.Fatalf("NewRequest(%q): %v", raw, err)
+	}
+	return Follow(releaseHosts...)(req, make([]*http.Request, hops))
+}
+
+// One case per field, because a guard that stopped reading the scheme would
+// still pass a test that only ever handed it another hostname.
+func TestFollowRefusesEachUnsafeShape(t *testing.T) {
+	for _, tc := range []struct{ name, url string }{
+		{"a plain-HTTP downgrade", "http://dl.reasonix.io/studio/x.tar.gz"},
+		{"credentials in the authority", "https://user:[email protected]/x"},
+		{"a port, which no release host answers on", "https://dl.reasonix.io:8443/x"},
+		{"a host that merely ends in ours", "https://dl.reasonix.io.evil.test/x"},
+		{"an unrelated host", "https://evil.test/x"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			err := hop(t, tc.url, 1)
+			if err == nil {
+				t.Fatalf("Follow(%q) = nil, want a refusal", tc.url)
+			}
+			// The identity, not the sentence: a caller telling a refusal from a
+			// transport failure has to reach it through errors.Is.
+			if !errors.Is(err, ErrRefused) {
+				t.Fatalf("Follow(%q) = %v, which does not carry ErrRefused", tc.url, err)
+			}
+		})
+	}
+}
+
+func TestFollowStopsAChain(t *testing.T) {
+	if err := hop(t, "https://dl.reasonix.io/x", maxHops); err == nil {
+		t.Fatalf("a chain of %d hops was followed; want it stopped", maxHops)
+	}
+}
+
+func TestFollowRefusesARequestWithNoTarget(t *testing.T) {
+	if err := Follow(releaseHosts...)(nil, nil); !errors.Is(err, ErrRefused) {
+		t.Fatal("a redirect with no request at all was allowed through")
+	}
+}
+
+// The hosts releases actually come from. A guard that refused these would break
+// every download rather than protect one.
+func TestFollowPassesTheHostsReleasesComeFrom(t *testing.T) {
+	for _, raw := range []string{
+		"https://dl.reasonix.io/studio/versions.json",
+		"https://reasonix.io/studio/x",
+		"https://github.com/esengine/DeepSeek-Reasonix/releases/download/v1/x",
+		"https://objects.githubusercontent.com/blob/x",
+	} {
+		if err := hop(t, raw, 1); err != nil {
+			t.Fatalf("Follow(%q) = %v, want it followed", raw, err)
+		}
+	}
+}
+
+// A resolver accepts both of these for the same name; a string comparison
+// accepts neither, and the refusal would look like an attack rather than a URL
+// that spelled its host unusually.
+func TestFollowComparesHostsTheWayDNSDoes(t *testing.T) {
+	for _, raw := range []string{"https://DL.Reasonix.IO/x", "https://dl.reasonix.io./x"} {
+		if err := hop(t, raw, 1); err != nil {
+			t.Fatalf("Follow(%q) = %v, want the same name matched", raw, err)
+		}
+	}
+}
+
+// An empty list is the shape a caller lands on by passing a slice it never
+// filled; it must refuse everything rather than wave everything through.
+func TestFollowWithNoHostsTrustsNothing(t *testing.T) {
+	req, err := http.NewRequest(http.MethodGet, "https://github.com/x", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := Follow()(req, nil); !errors.Is(err, ErrRefused) {
+		t.Fatal("a guard with no trusted hosts followed a redirect anyway")
+	}
+}

internal/releaseasset/cli.go

@@ -20,6 +20,8 @@ import (
 	"path"
 	"regexp"
 	"strings"
+
+	"reasonix/internal/redirectguard"
 )
 
 const (
@@ -58,7 +60,7 @@ func downloadCLIFromBase(ctx context.Context, client *http.Client, base, version
 
 	copyOfClient := *client
 	if official {
-		copyOfClient.CheckRedirect = validateOfficialRedirect
+		copyOfClient.CheckRedirect = redirectguard.Follow(officialHosts...)
 	}
 	archive, err := fetchBounded(ctx, &copyOfClient, archiveURL, maxCLIArchiveBytes)
 	if err != nil {
@@ -226,16 +228,7 @@ func extractCLITarGz(archive []byte, executable string) ([]byte, error) {
 	return binary, nil
 }
 
-func validateOfficialRedirect(req *http.Request, via []*http.Request) error {
-	if len(via) >= 10 {
-		return errors.New("remote CLI download stopped after 10 redirects")
-	}
-	if req == nil || req.URL == nil || !strings.EqualFold(req.URL.Scheme, "https") || req.URL.User != nil || req.URL.Port() != "" {
-		return errors.New("remote CLI download refused an unsafe redirect")
-	}
-	host := strings.ToLower(strings.TrimSuffix(req.URL.Hostname(), "."))
-	if host != "github.com" && !strings.HasSuffix(host, ".githubusercontent.com") {
-		return fmt.Errorf("remote CLI download refused redirect host %q", req.URL.Host)
-	}
-	return nil
-}
+// officialHosts is this path's own trust decision, not a shared list. That the
+// CLI upgrade happens to trust the same names is a coincidence of where we
+// publish; merging them would let one download's decision move the other's.
+var officialHosts = []string{"github.com", ".githubusercontent.com"}

internal/update/redirect_test.go

@@ -4,51 +4,30 @@ import (
 	"errors"
 	"net/http"
 	"testing"
+
+	"reasonix/internal/redirectguard"
 )
 
-func redirectTo(t *testing.T, raw string, hops int) error {
-	t.Helper()
-	req, err := http.NewRequest(http.MethodGet, raw, nil)
-	if err != nil {
-		t.Fatalf("NewRequest(%q): %v", raw, err)
-	}
-	return TrustedRedirect(req, make([]*http.Request, hops))
-}
-
-// Every field this guard reads is one an attacker controls once a release host
-// answers with a redirect, so each is stated as its own case rather than as one
-// "bad URL" — a guard that stopped checking the scheme would still pass a test
-// that only ever handed it another hostname.
-func TestTrustedRedirectRefusesWhatItWasWrittenFor(t *testing.T) {
-	for _, tc := range []struct{ name, url string }{
-		{"a plain-HTTP downgrade", "http://dl.reasonix.io/studio/x.tar.gz"},
-		{"credentials smuggled in the authority", "https://user:[email protected]/x"},
-		{"a port, which no release host answers on", "https://dl.reasonix.io:8443/x"},
-		{"a host that only looks like ours", "https://dl.reasonix.io.evil.test/x"},
-		{"an unrelated host outright", "https://evil.test/x"},
+// The mechanism is held by internal/redirectguard. What this holds is the
+// decision made here: Studio's own CDN is trusted, and a name that merely ends
+// in ours is not.
+func TestUpdateTrustsItsOwnReleaseHosts(t *testing.T) {
+	follow := redirectguard.Follow(releaseHosts...)
+	for _, tc := range []struct {
+		url  string
+		want bool
+	}{
+		{"https://dl.reasonix.io/studio/versions.json", true},
+		{"https://objects.githubusercontent.com/blob/x", true},
+		{"https://dl.reasonix.io.evil.test/x", false},
+		{"http://dl.reasonix.io/x", false},
 	} {
-		t.Run(tc.name, func(t *testing.T) {
-			if err := redirectTo(t, tc.url, 1); err == nil {
-				t.Fatalf("TrustedRedirect(%q) = nil, want a refusal", tc.url)
-			}
-		})
-	}
-	if err := redirectTo(t, "https://dl.reasonix.io/x", 10); err == nil {
-		t.Fatal("a chain of ten hops was followed; want it stopped")
-	}
-}
-
-// The hosts releases are actually served from. A guard that refused these would
-// break every update rather than protect one.
-func TestTrustedRedirectPassesTheReleaseHosts(t *testing.T) {
-	for _, raw := range []string{
-		"https://dl.reasonix.io/studio/versions.json",
-		"https://reasonix.io/studio/x",
-		"https://github.com/esengine/DeepSeek-Reasonix/releases/download/v1/x",
-		"https://objects.githubusercontent.com/blob/x",
-	} {
-		if err := redirectTo(t, raw, 1); err != nil {
-			t.Fatalf("TrustedRedirect(%q) = %v, want it followed", raw, err)
+		req, err := http.NewRequest(http.MethodGet, tc.url, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if followed := follow(req, nil) == nil; followed != tc.want {
+			t.Fatalf("follow(%q) followed=%v, want %v", tc.url, followed, tc.want)
 		}
 	}
 }

internal/update/transport.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"strings"
 	"time"
 )
 
@@ -279,36 +278,3 @@ func (p *progressReader) Read(b []byte) (int, error) {
 	}
 	return n, err
 }
-
-// TrustedRedirect is the CheckRedirect an update client must use: HTTPS only,
-// no userinfo, no port, and only hosts that serve our releases. Without it a
-// redirect could walk an authenticated download onto an attacker's host.
-func TrustedRedirect(req *http.Request, via []*http.Request) error {
-	if len(via) >= 10 {
-		return errors.New("update: stopped after 10 redirects")
-	}
-	if req == nil || req.URL == nil {
-		return errors.New("update: redirect has no target URL")
-	}
-	if !strings.EqualFold(req.URL.Scheme, "https") {
-		return fmt.Errorf("update: refusing redirect to non-HTTPS URL %q", req.URL.String())
-	}
-	if req.URL.Hostname() == "" {
-		return fmt.Errorf("update: refusing redirect without a hostname %q", req.URL.String())
-	}
-	if req.URL.User != nil {
-		return fmt.Errorf("update: refusing redirect with userinfo %q", req.URL.String())
-	}
-	if req.URL.Port() != "" || !trustedReleaseHost(req.URL.Hostname()) {
-		return fmt.Errorf("update: refusing redirect to untrusted host %q", req.URL.Host)
-	}
-	return nil
-}
-
-func trustedReleaseHost(host string) bool {
-	host = strings.ToLower(strings.TrimSuffix(strings.TrimSpace(host), "."))
-	return host == "reasonix.io" ||
-		strings.HasSuffix(host, ".reasonix.io") ||
-		host == "github.com" ||
-		strings.HasSuffix(host, ".githubusercontent.com")
-}

internal/update/updater.go

@@ -5,6 +5,8 @@ import (
 	"net/http"
 	"strings"
 	"time"
+
+	"reasonix/internal/redirectguard"
 )
 
 // ProgressFunc reports bytes received against the total, for a progress bar.
@@ -52,6 +54,12 @@ func New(opts Options) *Updater {
 	return &Updater{opts: opts}
 }
 
+// releaseHosts is where Studio's own artifacts are published: the catalog and
+// the assets on our CDN, and the GitHub pages a release redirects through.
+// This is the updater's trust decision and is deliberately not shared with the
+// CLI's, which happens to name a subset of the same hosts.
+var releaseHosts = []string{"reasonix.io", ".reasonix.io", "github.com", ".githubusercontent.com"}
+
 // guarded copies rather than mutates: the shells hand in a client the rest of
 // the app shares. A caller that already chose a policy keeps it.
 func guarded(c *http.Client) *http.Client {
@@ -59,7 +67,7 @@ func guarded(c *http.Client) *http.Client {
 		return c
 	}
 	copyOfClient := *c
-	copyOfClient.CheckRedirect = TrustedRedirect
+	copyOfClient.CheckRedirect = redirectguard.Follow(releaseHosts...)
 	return &copyOfClient
 }
 

tools/covergate/baseline.json

@@ -5,6 +5,7 @@
   "internal/permission/**": 90.2,
   "internal/plugin/**": 77.4,
   "internal/pluginpkg/**": 82.7,
+  "internal/redirectguard/**": 92.6,
   "internal/sandbox/**": 73.5,
   "internal/shellparse/**": 64.5,
   "internal/shellsafe/**": 61.1

tools/repolint/layers.go

@@ -21,6 +21,8 @@ var frontends = []string{
 
 // Utility layer: these packages carry no knowledge of the kernel and must
 // stay importable from anywhere without dragging a dependency graph along.
+// One may reach another: the layer is closed, so that drags nothing in, and
+// refusing it is what leaves the same judgement copied into several of them.
 var leaves = []string{
 	"internal/ablation",
 	"internal/agentgraph",
@@ -39,6 +41,7 @@ var leaves = []string{
 	"internal/nilutil",
 	"internal/planmode",
 	"internal/proc",
+	"internal/redirectguard",
 	"internal/releaseasset",
 	"internal/retrieval",
 	"internal/shellparse",
@@ -80,7 +83,7 @@ func checkLayering(imports map[string][]importRef) []Finding {
 
 func violates(pkg, dep string) string {
 	switch {
-	case matches(leaves, pkg):
+	case matches(leaves, pkg) && !matches(leaves, dep):
 		return fmt.Sprintf("%s is a utility-layer package and must not import %s", pkg, dep)
 	case under(dep, "internal/control") && !matches(frontends, pkg) && !under(pkg, "internal/control") && !host(pkg):
 		return fmt.Sprintf("%s may not import %s: the controller is reachable from frontends and entrypoints only", pkg, dep)

tools/repolint/layers_test.go

@@ -8,6 +8,7 @@ func TestLayeringContract(t *testing.T) {
 		wantViolation  bool
 	}{
 		{"utility package stays a leaf", "internal/fileutil", "internal/config", true},
+		{"a utility may use another utility", "internal/fileutil", "internal/textutil", false},
 		{"utility package may use the stdlib only", "internal/textutil", "internal/agent", true},
 		{"kernel may not reach the controller", "internal/agent", "internal/control", true},
 		{"kernel may not reach a frontend", "internal/agent", "internal/cli", true},