merge fix/leftover-tails: per-client link host, placeholder sanitizing, baseURL guard

2026-10-23 00:39:48 +00:00
parent 3a02211555 3413ccca9f
commit 460b314786
7 changed files with 404 additions and 10 deletions
@@ -151,11 +151,26 @@ func (r *Remote) httpClient() (*http.Client, error) {
 	return r.client, r.clientErr
 }

+// isAgentPlaceholderAddress reports whether addr is the placeholder the
+// node form used to store for agent nodes ("agent"). Agent nodes dial out
+// to the master and expose no panel endpoint; the write path no longer
+// stores the placeholder (see sanitizeAgentEndpoint in the node service),
+// but rows written earlier still carry it, and "agent" is a syntactically
+// valid hostname — without this guard baseURL would splice it into a
+// dialable URL (https://agent:2053/...) and do() would fire real requests
+// at whatever "agent" happens to resolve to.
+func isAgentPlaceholderAddress(addr string) bool {
+	return strings.EqualFold(addr, "agent")
+}
+
 func (r *Remote) baseURL() (string, error) {
 	addr, err := netsafe.NormalizeHost(r.node.Address)
 	if err != nil {
 		return "", err
 	}
+	if isAgentPlaceholderAddress(addr) {
+		return "", errors.New("node has no panel endpoint (agent-managed nodes cannot be dialed)")
+	}
 	scheme := r.node.Scheme
 	if scheme != "http" && scheme != "https" {
 		scheme = "https"
@@ -273,6 +273,33 @@ func TestRemoteBaseURL(t *testing.T) {
 	}
 }

+// Legacy agent-node rows store the node form's placeholder pair
+// (address "agent", port 2053). "agent" is a syntactically valid
+// hostname, so without a guard baseURL would splice it into a dialable
+// URL — https://agent:2053/... — and do() would fire real requests at
+// whatever "agent" resolves to. The placeholder must be refused with a
+// clear endpoint error instead, in any casing.
+func TestRemoteBaseURL_RejectsAgentPlaceholder(t *testing.T) {
+	for _, addr := range []string{"agent", "Agent", "AGENT"} {
+		r := NewRemote(&model.Node{Address: addr, Port: 2053, Scheme: "https"}, nil)
+		got, err := r.baseURL()
+		if err == nil || !strings.Contains(err.Error(), "no panel endpoint") {
+			t.Fatalf("baseURL(%q) = %q, %v; want a panel-endpoint error", addr, got, err)
+		}
+	}
+}
+
+// The latent trigger: a placeholder row that also carries a panel API
+// token passes do()'s credential gate and reaches baseURL. The request
+// must die there — never on the network.
+func TestRemoteDo_AgentPlaceholderNeverDials(t *testing.T) {
+	r := NewRemote(&model.Node{Address: "agent", Port: 2053, Scheme: "https", ApiToken: "tok"}, nil)
+	_, err := r.do(context.Background(), http.MethodGet, "/probe", nil)
+	if err == nil || !strings.Contains(err.Error(), "no panel endpoint") {
+		t.Fatalf("do() error = %v, want a panel-endpoint error", err)
+	}
+}
+
 func TestIsNonEmptySlice(t *testing.T) {
 	cases := []struct {
 		name string
@@ -435,18 +435,32 @@ func normalizeBasePath(p string) string {
 }

 func (s *NodeService) normalize(n *model.Node) error {
+	return s.normalizeMode(n, false)
+}
+
+// normalizeMode validates a node row before it is stored. agentEndpoint
+// marks an agent-node write: agent nodes have no panel endpoint to dial,
+// so an unset address/port pair (the form placeholder, sanitized away by
+// sanitizeAgentEndpoint) is legitimate and skips the host/port checks.
+// Any value that IS present is validated exactly like a traditional
+// node's.
+func (s *NodeService) normalizeMode(n *model.Node, agentEndpoint bool) error {
 	n.Name = strings.TrimSpace(n.Name)
 	n.ApiToken = strings.TrimSpace(n.ApiToken)
 	if n.Name == "" {
 		return common.NewError("node name is required")
 	}
-	addr, err := netsafe.NormalizeHost(n.Address)
-	if err != nil {
-		return common.NewError(err.Error())
+	if !agentEndpoint || n.Address != "" {
+		addr, err := netsafe.NormalizeHost(n.Address)
+		if err != nil {
+			return common.NewError(err.Error())
+		}
+		n.Address = addr
 	}
-	n.Address = addr
-	if n.Port <= 0 || n.Port > 65535 {
-		return common.NewError("node port must be 1-65535")
+	if !agentEndpoint || n.Port != 0 {
+		if n.Port <= 0 || n.Port > 65535 {
+			return common.NewError("node port must be 1-65535")
+		}
 	}
 	if n.Scheme != "http" && n.Scheme != "https" {
 		n.Scheme = "https"
@@ -487,7 +501,11 @@ func (s *NodeService) normalize(n *model.Node) error {
 }

 func (s *NodeService) Create(n *model.Node) error {
-	if err := s.normalize(n); err != nil {
+	return s.create(n, false)
+}
+
+func (s *NodeService) create(n *model.Node, agentEndpoint bool) error {
+	if err := s.normalizeMode(n, agentEndpoint); err != nil {
 		return err
 	}
 	db := database.GetDB()
@@ -520,13 +538,40 @@ func (s *NodeService) CreateFromRequest(req *NodeMutationRequest) (*NodeView, er
 	if err := req.validateCredentials(true); err != nil {
 		return nil, err
 	}
+	isAgent := req.NodeType == "agent"
+	if isAgent {
+		// A brand-new agent node has no binding yet, so no resolved
+		// address can be known: the placeholder sanitizes to empty and
+		// is filled in by later writes once the agent reports.
+		sanitizeAgentEndpoint(req, "")
+	}
 	n := req.toNode()
-	if err := s.Create(n); err != nil {
+	if err := s.create(n, isAgent); err != nil {
 		return nil, err
 	}
 	return toNodeView(n), nil
 }

+// resolvedAgentAddress returns the externally usable address of the agent
+// bound to a node (latest binding wins, via ResolveAgentAddresses), or ""
+// when the node has no bound agent or the agent never reported one. The
+// stored row of an agent node carries this value (or empty) instead of
+// the create form's placeholder — see sanitizeAgentEndpoint.
+func (s *NodeService) resolvedAgentAddress(nodeID int) string {
+	var ag model.AgentNode
+	if err := database.GetDB().
+		Where("node_id = ?", nodeID).
+		Order("last_seen DESC").
+		First(&ag).Error; err != nil {
+		return ""
+	}
+	v4, v6 := ResolveAgentAddresses(&ag)
+	if v4 != "" {
+		return v4
+	}
+	return v6
+}
+
 func (s *NodeService) Update(id int, in *model.Node) error {
 	if err := s.normalize(in); err != nil {
 		return err
@@ -583,8 +628,12 @@ func (s *NodeService) UpdateFromRequest(id int, req *NodeMutationRequest) error
 	if err := req.validateCredentials(false); err != nil {
 		return err
 	}
+	isAgent := req.NodeType == "agent" || s.IsAgentManaged(id)
+	if isAgent {
+		sanitizeAgentEndpoint(req, s.resolvedAgentAddress(id))
+	}
 	in := req.toNode()
-	if err := s.normalize(in); err != nil {
+	if err := s.normalizeMode(in, isAgent); err != nil {
 		return err
 	}
 	inboundTagsJSON, err := json.Marshal(in.InboundTags)
@@ -608,7 +657,6 @@ func (s *NodeService) UpdateFromRequest(id int, req *NodeMutationRequest) error
 	}
 	// Agent nodes authenticate with their agent token, never a panel API
 	// token — the requirement below must not make them uneditable.
-	isAgent := req.NodeType == "agent" || s.IsAgentManaged(id)
 	if !isAgent && apiToken == "" && in.Enable && in.TlsVerifyMode != "mtls" {
 		return common.NewError("apiToken is required unless mtls is enabled")
 	}
@@ -0,0 +1,152 @@
+package service
+
+import (
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+)
+
+// Agent nodes have no panel endpoint: the node form submits the
+// placeholder pair address="agent", port=2053 for them. The write paths
+// must never store that pair — an agent node's row carries its resolved
+// address when one is known, or empty when it is not — while traditional
+// nodes keep the strict endpoint validation they always had.
+
+func strPtr(s string) *string { return &s }
+
+func TestCreateFromRequest_AgentPlaceholderNotStored(t *testing.T) {
+	setupConflictDB(t)
+	svc := &NodeService{}
+
+	view, err := svc.CreateFromRequest(&NodeMutationRequest{
+		Name: "agent-new", Enable: true, Scheme: "https",
+		Address: "agent", Port: 2053, NodeType: "agent",
+	})
+	if err != nil {
+		t.Fatalf("CreateFromRequest(agent): %v", err)
+	}
+	if view.Address == "agent" || view.Port == 2053 {
+		t.Fatalf("create view still carries the placeholder: %+v", view)
+	}
+	stored, err := svc.GetById(view.Id)
+	if err != nil {
+		t.Fatalf("GetById: %v", err)
+	}
+	// A brand-new agent node has no binding, hence no resolved address:
+	// the honest stored value is empty, port 0.
+	if stored.Address != "" || stored.Port != 0 {
+		t.Fatalf("stored agent endpoint = %q:%d, want empty:0", stored.Address, stored.Port)
+	}
+}
+
+func TestCreateFromRequest_TraditionalKeepsEndpoint(t *testing.T) {
+	setupConflictDB(t)
+	svc := &NodeService{}
+
+	view, err := svc.CreateFromRequest(&NodeMutationRequest{
+		Name: "trad-new", Enable: true, Scheme: "https",
+		Address: "198.51.100.10", Port: 8443, ApiToken: strPtr("tok-1"),
+	})
+	if err != nil {
+		t.Fatalf("CreateFromRequest(traditional): %v", err)
+	}
+	stored, err := svc.GetById(view.Id)
+	if err != nil {
+		t.Fatalf("GetById: %v", err)
+	}
+	if stored.Address != "198.51.100.10" || stored.Port != 8443 {
+		t.Fatalf("stored traditional endpoint = %q:%d, want 198.51.100.10:8443", stored.Address, stored.Port)
+	}
+}
+
+func TestCreateFromRequest_TraditionalEmptyAddressStillRejected(t *testing.T) {
+	setupConflictDB(t)
+	svc := &NodeService{}
+
+	if _, err := svc.CreateFromRequest(&NodeMutationRequest{
+		Name: "trad-bad", Enable: true, Scheme: "https", ApiToken: strPtr("tok-2"),
+	}); err == nil {
+		t.Fatal("traditional create with an empty address must still fail validation")
+	}
+}
+
+// seedAgentNodeRow stores a legacy placeholder row plus its agent
+// binding; ipv4 == "" models an agent that never reported an address.
+func seedAgentNodeRow(t *testing.T, name, ipv4 string) *model.Node {
+	t.Helper()
+	db := database.GetDB()
+	node := &model.Node{Name: name, Scheme: "https", Address: "agent", Port: 2053, Enable: true}
+	if err := db.Create(node).Error; err != nil {
+		t.Fatalf("create node: %v", err)
+	}
+	ag := &model.AgentNode{TokenHash: "hash-" + name, Name: name, NodeID: &node.Id, IPv4: ipv4, LastSeen: 100}
+	if err := db.Create(ag).Error; err != nil {
+		t.Fatalf("create binding: %v", err)
+	}
+	return node
+}
+
+func TestUpdateFromRequest_AgentPlaceholderReplacedByResolved(t *testing.T) {
+	setupConflictDB(t)
+	node := seedAgentNodeRow(t, "agent-upd", "203.0.113.9")
+	svc := &NodeService{}
+
+	// Exactly what the edit form submits for an agent node today.
+	err := svc.UpdateFromRequest(node.Id, &NodeMutationRequest{
+		Name: "agent-upd", Enable: true, Scheme: "https",
+		Address: "agent", Port: 2053, NodeType: "agent",
+	})
+	if err != nil {
+		t.Fatalf("UpdateFromRequest(agent): %v", err)
+	}
+	stored, err := svc.GetById(node.Id)
+	if err != nil {
+		t.Fatalf("GetById: %v", err)
+	}
+	if stored.Address != "203.0.113.9" || stored.Port != 0 {
+		t.Fatalf("stored agent endpoint = %q:%d, want resolved 203.0.113.9:0", stored.Address, stored.Port)
+	}
+}
+
+func TestUpdateFromRequest_AgentNoReport_SanitizesToEmpty(t *testing.T) {
+	setupConflictDB(t)
+	node := seedAgentNodeRow(t, "agent-silent", "")
+	svc := &NodeService{}
+
+	err := svc.UpdateFromRequest(node.Id, &NodeMutationRequest{
+		Name: "agent-silent", Enable: true, Scheme: "https",
+		Address: "agent", Port: 2053, NodeType: "agent",
+	})
+	if err != nil {
+		t.Fatalf("UpdateFromRequest(agent): %v", err)
+	}
+	stored, err := svc.GetById(node.Id)
+	if err != nil {
+		t.Fatalf("GetById: %v", err)
+	}
+	if stored.Address != "" || stored.Port != 0 {
+		t.Fatalf("stored agent endpoint = %q:%d, want empty:0 (no placeholder, no invented address)", stored.Address, stored.Port)
+	}
+}
+
+func TestUpdateFromRequest_AgentRealAddressKeptVerbatim(t *testing.T) {
+	setupConflictDB(t)
+	node := seedAgentNodeRow(t, "agent-real", "203.0.113.9")
+	svc := &NodeService{}
+
+	err := svc.UpdateFromRequest(node.Id, &NodeMutationRequest{
+		Name: "agent-real", Enable: true, Scheme: "https",
+		Address: "edge.example.com", Port: 443, NodeType: "agent",
+	})
+	if err != nil {
+		t.Fatalf("UpdateFromRequest(agent, real address): %v", err)
+	}
+	stored, err := svc.GetById(node.Id)
+	if err != nil {
+		t.Fatalf("GetById: %v", err)
+	}
+	if stored.Address != "edge.example.com" || stored.Port != 443 {
+		t.Fatalf("stored agent endpoint = %q:%d, want submitted edge.example.com:443", stored.Address, stored.Port)
+	}
+}
@@ -313,6 +313,31 @@ func (r *NodeMutationRequest) validateCredentials(create bool) error {
 	return nil
 }

+// isAgentPlaceholderAddress reports whether addr is the placeholder the
+// node form submits for agent nodes ("agent": they dial out to the master
+// and have no panel endpoint to store). Keep in sync with the guard in
+// internal/web/runtime's baseURL, which refuses to dial the placeholder
+// for rows written before this sanitization existed.
+func isAgentPlaceholderAddress(addr string) bool {
+	return strings.EqualFold(strings.TrimSpace(addr), "agent")
+}
+
+// sanitizeAgentEndpoint rewrites an agent-node write's endpoint fields so
+// the form placeholder never reaches the nodes table: a placeholder (or
+// empty) address is replaced by the node's currently resolved address —
+// "" when none is known yet (a new node, or an agent that never
+// reported) — and the port is dropped to 0, since an agent node has no
+// panel port either. A real address submitted for an agent node is kept
+// verbatim, port included.
+func sanitizeAgentEndpoint(req *NodeMutationRequest, resolved string) {
+	addr := strings.TrimSpace(req.Address)
+	if addr != "" && !isAgentPlaceholderAddress(addr) {
+		return
+	}
+	req.Address = resolved
+	req.Port = 0
+}
+
 func (r *NodeMutationRequest) toNode() *model.Node {
 	n := &model.Node{
 		Id:                  r.Id,