Test-quality audit: fix 2 prod bugs, strengthen weak tests, add mutation/fuzz/CI tooling (#5345)

* test(audit): add gremlins/rapid/coverage tooling + AUDIT.md scaffold * test(audit): hygiene sweep (race-clean except logger global; Finding #2) + smell inventory * test(audit): cover untested error/edge branches (TLS proxy+pin, migration tag cleanup=Finding #1) * test(audit): strengthen internal/sub link tests (dedup key, TLS/Reality mapping, clash well-formedness) * test(audit): property (rapid) + fuzz tests for joinHostPort/userinfo/pin/ParseLink * test(audit): tighten frontend subSortIndex rejection assertions + wire coverage * ci(audit): add shuffle gate + non-blocking race job (Finding #2) + fuzz-smoke; document mutation policy * chore(audit): gitignore frontend coverage output * test(audit): exhaustive whole-repo pass — strengthen 5 weak/fake tests (netproxy, CSP, modal per-protocol loops, schema coercions) * docs(contributing): add Testing section (conventions, race/shuffle, fuzz, mutation policy); drop AUDIT.md ledger * fix(logger,migration): guard logBuffer with mutex; execute legacy tag cleanup (tx.Exec); make CI race gate blocking * ci(mutation): add nightly scoped gremlins workflow (informational artifacts) * test(audit): strengthen runtime tests — baseURL scheme/port bounds, isNonEmptySlice, trafficReset * test(audit): strengthen clash tests — reality field mapping + tcp-header validation * test(audit): runtime — egress-proxy + content-type tests; drop redundant bp=='' branch * test(audit): strengthen link parser/helper tests (defaultPort, splitComma, base64, canonicalQuery, tls/reality/transport mapping) * test(audit): strengthen sub/xray/common/netsafe/mtproto/config/middleware tests (kill surviving mutants) * test(audit): raise timeout on protocol-iteration modal tests (heavy re-renders, slow on CI) * fix(logger): GetLogs returns at most c entries (off-by-one fix; addresses PR review) * perf(logger): snapshot logBuffer under lock so GetLogs doesn't block logging; clarify fuzz-seed docs (addresses PR review)
2026-08-24 11:57:15 +00:00 · 2026-06-15 15:17:03 +02:00
parent b5872af279
commit 7605902324
37 changed files with 2580 additions and 330 deletions
@@ -87,9 +87,6 @@ func (r *Remote) baseURL() (string, error) {
 		return "", fmt.Errorf("invalid node port %d", r.node.Port)
 	}
 	bp := r.node.BasePath
-	if bp == "" {
-		bp = "/"
-	}
 	if !strings.HasSuffix(bp, "/") {
 		bp += "/"
 	}
@@ -1,12 +1,20 @@
 package runtime

 import (
+	"context"
 	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"testing"

 	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
 )

+type stubEgress struct{ url string }
+
+func (s stubEgress) NodeEgressProxyURL(int) string { return s.url }
+
 // cacheGetTag must resolve a remote inbound id even when the n<id>- prefix
 // sits on only one side: the node may store the bare tag while the central
 // panel pushes the prefixed form, or vice versa. Without this a mismatch makes
@@ -50,6 +58,115 @@ func TestWireInboundIncludesShareAddressFields(t *testing.T) {
 	}
 }

+func TestRemoteHTTPClientEgressProxy(t *testing.T) {
+	// OutboundTag + a resolver → a dedicated proxy client (not the shared default).
+	withTag := NewRemote(&model.Node{Id: 1, Scheme: "https", TlsVerifyMode: "verify", OutboundTag: "warp"}, stubEgress{url: "socks5://127.0.0.1:1080"})
+	c, err := withTag.httpClient()
+	if err != nil {
+		t.Fatalf("httpClient: %v", err)
+	}
+	if c == defaultNodeHTTPClient {
+		t.Fatal("OutboundTag + resolver must produce a dedicated egress client, not the shared default")
+	}
+	// No OutboundTag → no egress proxy → shared default client (verify mode).
+	noTag := NewRemote(&model.Node{Id: 2, Scheme: "https", TlsVerifyMode: "verify"}, stubEgress{url: "socks5://127.0.0.1:1080"})
+	c2, err := noTag.httpClient()
+	if err != nil {
+		t.Fatalf("httpClient: %v", err)
+	}
+	if c2 != defaultNodeHTTPClient {
+		t.Fatal("no OutboundTag must use the shared default client")
+	}
+}
+
+func TestRemoteDoSetsContentType(t *testing.T) {
+	var gotCT string
+	srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotCT = r.Header.Get("Content-Type")
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"success":true}`))
+	}))
+	defer srv.Close()
+
+	r := NewRemote(nodeForServer(t, srv, "skip", ""), nil)
+	if _, err := r.do(context.Background(), http.MethodPost, "x", url.Values{"a": {"b"}}); err != nil {
+		t.Fatalf("do: %v", err)
+	}
+	if gotCT != "application/x-www-form-urlencoded" {
+		t.Fatalf("Content-Type = %q, want application/x-www-form-urlencoded", gotCT)
+	}
+}
+
+func TestRemoteBaseURL(t *testing.T) {
+	cases := []struct {
+		name    string
+		scheme  string
+		port    int
+		bp      string
+		want    string
+		wantErr bool
+	}{
+		{"https default path", "https", 443, "", "https://example.com:443/", false},
+		{"http custom path gets trailing slash", "http", 8080, "/panel", "http://example.com:8080/panel/", false},
+		{"empty scheme defaults to https", "", 2096, "/", "https://example.com:2096/", false},
+		{"invalid scheme defaults to https", "ftp", 2096, "/", "https://example.com:2096/", false},
+		{"port zero rejected", "https", 0, "/", "", true},
+		{"port above range rejected", "https", 65536, "/", "", true},
+		{"negative port rejected", "https", -1, "/", "", true},
+		{"max port accepted", "https", 65535, "/", "https://example.com:65535/", false},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			r := NewRemote(&model.Node{Address: "example.com", Scheme: c.scheme, Port: c.port, BasePath: c.bp}, nil)
+			got, err := r.baseURL()
+			if c.wantErr {
+				if err == nil {
+					t.Fatalf("expected error for scheme=%q port=%d", c.scheme, c.port)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if got != c.want {
+				t.Fatalf("baseURL = %q, want %q", got, c.want)
+			}
+		})
+	}
+}
+
+func TestIsNonEmptySlice(t *testing.T) {
+	cases := []struct {
+		name string
+		in   any
+		want bool
+	}{
+		{"non-empty slice", []any{1}, true},
+		{"empty slice", []any{}, false},
+		{"nil slice", []any(nil), false},
+		{"not a slice", "x", false},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := isNonEmptySlice(c.in); got != c.want {
+				t.Fatalf("isNonEmptySlice(%#v) = %v, want %v", c.in, got, c.want)
+			}
+		})
+	}
+}
+
+func TestWireInboundTrafficReset(t *testing.T) {
+	with := wireInbound(&model.Inbound{TrafficReset: "daily"})
+	if got := with.Get("trafficReset"); got != "daily" {
+		t.Fatalf("trafficReset = %q, want daily", got)
+	}
+	// Empty TrafficReset must be omitted entirely, not sent as an empty field.
+	without := wireInbound(&model.Inbound{})
+	if without.Has("trafficReset") {
+		t.Fatalf("trafficReset must be omitted when empty, got %q", without.Get("trafficReset"))
+	}
+}
+
 func TestWireInboundDefaultsShareAddressStrategy(t *testing.T) {
 	values := wireInbound(&model.Inbound{})

@@ -0,0 +1,73 @@
+package runtime
+
+import (
+	"bytes"
+	"crypto/sha256"
+	"encoding/base64"
+	"encoding/hex"
+	"strings"
+	"testing"
+
+	"pgregory.net/rapid"
+)
+
+func insertColons(h string) string {
+	var b strings.Builder
+	for i := 0; i < len(h); i += 2 {
+		if i > 0 {
+			b.WriteByte(':')
+		}
+		b.WriteString(h[i : i+2])
+	}
+	return b.String()
+}
+
+// TestProp_DecodeCertPin_FormatAgnostic asserts that for ANY 32-byte pin, every
+// accepted encoding (hex lower/upper, openssl colon-hex, base64 std/raw/url) decodes
+// back to the same bytes. Generalizes the fixed-input TestDecodeCertPin so a mutant
+// that breaks one decoding path is caught across the whole input space.
+func TestProp_DecodeCertPin_FormatAgnostic(t *testing.T) {
+	rapid.Check(t, func(t *rapid.T) {
+		raw := rapid.SliceOfN(rapid.Byte(), sha256.Size, sha256.Size).Draw(t, "raw")
+		hx := hex.EncodeToString(raw)
+		forms := []string{
+			hx,
+			strings.ToUpper(hx),
+			insertColons(hx),
+			base64.StdEncoding.EncodeToString(raw),
+			base64.RawStdEncoding.EncodeToString(raw),
+			base64.URLEncoding.EncodeToString(raw),
+			base64.RawURLEncoding.EncodeToString(raw),
+		}
+		for _, f := range forms {
+			got, err := DecodeCertPin(f)
+			if err != nil {
+				t.Fatalf("DecodeCertPin(%q) errored: %v", f, err)
+			}
+			if !bytes.Equal(got, raw) {
+				t.Fatalf("DecodeCertPin(%q) = %x, want %x", f, got, raw)
+			}
+		}
+	})
+}
+
+// FuzzDecodeCertPin asserts the security-load-bearing decoder never panics, never
+// returns a non-32-byte slice with a nil error, and never returns bytes alongside an
+// error. Seeded from the known-good/known-bad cases.
+func FuzzDecodeCertPin(f *testing.F) {
+	seed := sha256.Sum256([]byte("seed"))
+	f.Add(hex.EncodeToString(seed[:]))
+	f.Add(base64.StdEncoding.EncodeToString(seed[:]))
+	f.Add(insertColons(hex.EncodeToString(seed[:])))
+	f.Add("")
+	f.Add("not-a-pin")
+	f.Fuzz(func(t *testing.T, s string) {
+		got, err := DecodeCertPin(s)
+		if err == nil && len(got) != sha256.Size {
+			t.Fatalf("DecodeCertPin(%q): nil error but %d bytes, want %d", s, len(got), sha256.Size)
+		}
+		if err != nil && got != nil {
+			t.Fatalf("DecodeCertPin(%q): error %v but returned bytes %x", s, err, got)
+		}
+	})
+}
@@ -116,8 +116,67 @@ func TestHTTPClientForNodeVerifyShared(t *testing.T) {
 }

 func TestHTTPClientForNodePinInvalid(t *testing.T) {
-	if _, err := HTTPClientForNode(&model.Node{Scheme: "https", TlsVerifyMode: "pin", PinnedCertSha256: "not-a-pin"}, ""); err == nil {
-		t.Fatal("expected error for invalid pin")
+	// pin mode must fail closed, and with a specific error per cause — not merely
+	// "some error" (which a bug anywhere in the build path would also satisfy).
+	cases := []struct {
+		name    string
+		pin     string
+		wantErr string
+	}{
+		{"garbage pin", "not-a-pin", "must be a SHA-256 hash"},
+		{"empty pin", "", "certificate pin is empty"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			_, err := HTTPClientForNode(&model.Node{Scheme: "https", TlsVerifyMode: "pin", PinnedCertSha256: c.pin}, "")
+			if err == nil {
+				t.Fatalf("expected error for pin %q", c.pin)
+			}
+			if !strings.Contains(err.Error(), c.wantErr) {
+				t.Fatalf("error = %q, want it to contain %q", err.Error(), c.wantErr)
+			}
+		})
+	}
+}
+
+// TestHTTPClientForNode_ProxyPinPreservesPinEnforcement covers the proxy+pin branch
+// (tls_client.go:43-52): when a node uses a proxy AND pin mode, the proxy client's
+// transport must carry the pinning tls.Config (the `transport.TLSClientConfig = tlsCfg`
+// line). Dropping it would silently disable certificate pinning whenever a proxy is set.
+func TestHTTPClientForNode_ProxyPinPreservesPinEnforcement(t *testing.T) {
+	pin := base64.StdEncoding.EncodeToString(make([]byte, sha256.Size))
+	n := &model.Node{Scheme: "https", TlsVerifyMode: "pin", PinnedCertSha256: pin}
+
+	c, err := HTTPClientForNode(n, "socks5://127.0.0.1:1080")
+	if err != nil {
+		t.Fatalf("HTTPClientForNode: %v", err)
+	}
+	if c == defaultNodeHTTPClient {
+		t.Fatal("proxy client must not be the shared default client")
+	}
+	tr, ok := c.Transport.(*http.Transport)
+	if !ok {
+		t.Fatalf("transport is %T, want *http.Transport", c.Transport)
+	}
+	if tr.TLSClientConfig == nil || tr.TLSClientConfig.VerifyConnection == nil {
+		t.Fatal("pin mode over a proxy must install a pinning tls.Config (VerifyConnection); pin enforcement was dropped")
+	}
+}
+
+// TestHTTPClientForNode_ProxyVerifyNoPin covers the proxy+verify branch
+// (tls_client.go:40-42): verify mode over a proxy returns the proxy client as-is,
+// using system-CA verification and NOT a pin VerifyConnection.
+func TestHTTPClientForNode_ProxyVerifyNoPin(t *testing.T) {
+	n := &model.Node{Scheme: "https", TlsVerifyMode: "verify"}
+	c, err := HTTPClientForNode(n, "socks5://127.0.0.1:1080")
+	if err != nil {
+		t.Fatalf("HTTPClientForNode: %v", err)
+	}
+	if c == defaultNodeHTTPClient {
+		t.Fatal("proxy client must not be the shared default client")
+	}
+	if tr, ok := c.Transport.(*http.Transport); ok && tr.TLSClientConfig != nil && tr.TLSClientConfig.VerifyConnection != nil {
+		t.Fatal("verify mode must not install a pin VerifyConnection")
 	}
 }