Harden node lifecycle and API handling

.env.example

@@ -9,8 +9,9 @@ NODE_HOST = "0.0.0.0"
 SSL_CERT_FILE = /var/lib/pg-node/certs/ssl_cert.pem
 SSL_KEY_FILE = /var/lib/pg-node/certs/ssl_key.pem
 
-# api key must be a valid uuid (you can use any version you want)
-API_KEY = xxxxxxxx-yyyy-zzzz-mmmm-aaaaaaaaaaa
+# Required: generate a non-zero UUID (for example, with `uuidgen`) and configure
+# the same value in the panel. Placeholder and zero UUID values are rejected.
+API_KEY = 01234567-89ab-4def-8123-456789abcdef
 
 ### can be rest or grpc
 # SERVICE_PROTOCOL = grpc
@@ -20,6 +21,7 @@ API_KEY = xxxxxxxx-yyyy-zzzz-mmmm-aaaaaaaaaaa
 # GENERATED_CONFIG_PATH = /var/lib/pg-node/generated
 # LOG_BUFFER_SIZE = 10000
 # STARTUP_LOG_TAIL_SIZE = 200
+# Both statistics intervals must be positive integers when overridden.
 # STATS_UPDATE_INTERVAL_SECONDS = 10
 # STATS_CLEANUP_INTERVAL_SECONDS = 300
 

README.md

@@ -17,6 +17,21 @@
 # Documentation
 You can find a full guide in docs https://docs.pasarguard.org/en/node/
 
+## Upgrade notes
+
+This release makes startup configuration validation strict:
+
+- `API_KEY` is required and must be a non-zero UUID. Generate a new value for
+  each node (for example, with `uuidgen`) and configure the same value in the
+  panel before restarting the node. The placeholder from older `.env` files is
+  not valid.
+- `STATS_UPDATE_INTERVAL_SECONDS` and `STATS_CLEANUP_INTERVAL_SECONDS`, when
+  set, must be positive integers. Remove old zero or negative overrides to use
+  the defaults (`10` and `300` seconds), or replace them with positive values.
+
+The node now exits during startup with a configuration error instead of running
+with an unauthenticated key or invalid statistics intervals.
+
 # One-Click Installation (Recommended)
 The easiest way to install PasarGuard Node is using our automated installation script:
 

backend/xray/config.go

@@ -307,6 +307,13 @@ func (i *Inbound) removeUser(email string) {
 	}
 }
 
+func (i *Inbound) hasUser(email string) bool {
+	i.mu.RLock()
+	defer i.mu.RUnlock()
+	_, ok := i.clients[email]
+	return ok
+}
+
 type Stats struct{}
 
 func (c *Config) ToBytes() ([]byte, error) {

backend/xray/user.go

@@ -6,13 +6,28 @@ import (
 	"fmt"
 	"log"
 	"slices"
+	"sort"
 	"strings"
 	"time"
 
 	"github.com/pasarguard/node/backend/xray/api"
 	"github.com/pasarguard/node/common"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
+type inboundUserHandler interface {
+	AddInboundUser(context.Context, string, api.Account) error
+	RemoveInboundUser(context.Context, string, string) error
+}
+
+func (x *Xray) inboundUserHandler() inboundUserHandler {
+	if x.userHandler != nil {
+		return x.userHandler
+	}
+	return x.handler
+}
+
 func setupUserAccount(user *common.User) (api.ProxySettings, error) {
 	settings := api.ProxySettings{}
 	if user.GetProxies().GetVmess() != nil {
@@ -43,6 +58,33 @@ func setupUserAccount(user *common.User) (api.ProxySettings, error) {
 	return settings, nil
 }
 
+// removeInboundUser treats an already-absent runtime user as a successful
+// idempotent revoke, but never hides transport/core errors. A caller must not
+// report a revoked credential while Xray still accepts it.
+func removeInboundUser(ctx context.Context, handler inboundUserHandler, inbound *Inbound, email string) error {
+	err := handler.RemoveInboundUser(ctx, inbound.Tag, email)
+	if isBenignUserRemovalError(err, inbound, email) {
+		return nil
+	}
+	return err
+}
+
+func isBenignUserRemovalError(err error, inbound *Inbound, email string) bool {
+	if err == nil || status.Code(err) == codes.NotFound {
+		return true
+	}
+	// Xray sometimes collapses an absent-user result into Unknown. Only accept
+	// that ambiguous status when it is a real gRPC status and the authoritative
+	// inbound snapshot already confirms that no credential can be resurrected.
+	// This deliberately avoids message matching: transport/core failures and an
+	// Unknown result for a cached user remain actionable errors.
+	grpcStatus, ok := status.FromError(err)
+	if !ok || grpcStatus.Code() != codes.Unknown || inbound == nil {
+		return false
+	}
+	return !inbound.hasUser(email)
+}
+
 func inboundFlow(inbound *Inbound) string {
 	if inbound == nil || inbound.Settings == nil {
 		return ""
@@ -128,7 +170,7 @@ func (x *Xray) SyncUser(ctx context.Context, user *common.User) error {
 		return err
 	}
 
-	handler := x.handler
+	handler := x.inboundUserHandler()
 	inbounds := x.config.InboundConfigs
 
 	var errMessage strings.Builder
@@ -140,17 +182,22 @@ func (x *Xray) SyncUser(ctx context.Context, user *common.User) error {
 			continue
 		}
 
-		_ = handler.RemoveInboundUser(ctx, inbound.Tag, user.Email)
+		if err := removeInboundUser(ctx, handler, inbound, user.Email); err != nil {
+			return fmt.Errorf("failed to remove user %q from inbound %q: %w", user.Email, inbound.Tag, err)
+		}
+		// Keep the restart snapshot aligned with every confirmed runtime
+		// mutation. In particular, a failed replacement must not leave the old
+		// credential in the snapshot where a health restart could resurrect it.
+		inbound.removeUser(user.Email)
 		account, isActive := isActiveInbound(inbound, userInbounds, proxySetting)
 		if isActive {
-			inbound.updateUser(account)
 			err = handler.AddInboundUser(ctx, inbound.Tag, accountForAPI(inbound, account))
 			if err != nil {
 				log.Println(err)
 				errMessage.WriteString("\n" + err.Error())
+			} else {
+				inbound.updateUser(account)
 			}
-		} else {
-			inbound.removeUser(user.GetEmail())
 		}
 	}
 
@@ -218,34 +265,45 @@ func (x *Xray) UpdateUsers(ctx context.Context, users []*common.User) error {
 	x.syncMu.Lock()
 	defer x.syncMu.Unlock()
 
-	handler := x.handler
+	handler := x.inboundUserHandler()
 	inboundByTag, updates := x.config.buildInboundUpdates(users)
-	var errMessage strings.Builder
+	var updateErrors []error
 
 	for tag, update := range updates {
 		removeEmails := make([]string, 0, len(update.removeEmailSet))
 		for email := range update.removeEmailSet {
 			removeEmails = append(removeEmails, email)
 		}
+		sort.Strings(removeEmails)
 
 		inbound := inboundByTag[tag]
-		inbound.updateUsers(update.accounts, removeEmails)
 
 		for _, email := range removeEmails {
-			handler.RemoveInboundUser(ctx, tag, email)
+			if err := removeInboundUser(ctx, handler, inbound, email); err != nil {
+				updateErrors = append(updateErrors, fmt.Errorf("failed to remove user %q from inbound %q: %w", email, tag, err))
+				continue
+			}
+			inbound.removeUser(email)
 		}
 
 		for _, account := range update.accounts {
-			_ = handler.RemoveInboundUser(ctx, tag, account.GetEmail())
+			email := account.GetEmail()
+			if err := removeInboundUser(ctx, handler, inbound, email); err != nil {
+				updateErrors = append(updateErrors, fmt.Errorf("failed to replace user %q in inbound %q: %w", email, tag, err))
+				continue
+			}
+			inbound.removeUser(email)
 			if err := handler.AddInboundUser(ctx, tag, accountForAPI(inbound, account)); err != nil {
 				log.Println(err)
-				errMessage.WriteString("\n" + err.Error())
+				updateErrors = append(updateErrors, fmt.Errorf("failed to add user %q to inbound %q: %w", email, tag, err))
+				continue
 			}
+			inbound.updateUser(account)
 		}
 	}
 
-	if errMessage.String() != "" {
-		return errors.New("failed to update users:" + errMessage.String())
+	if len(updateErrors) > 0 {
+		return fmt.Errorf("failed to update users: %w", errors.Join(updateErrors...))
 	}
 
 	return nil

backend/xray/user_removal_test.go

@@ -0,0 +1,40 @@
+package xray
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/pasarguard/node/backend/xray/api"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+func TestIsBenignUserRemovalError(t *testing.T) {
+	absentInbound := &Inbound{clients: make(map[string]api.Account)}
+	for _, err := range []error{
+		nil,
+		status.Error(codes.NotFound, "user not found"),
+		status.Error(codes.Unknown, "proxy/trojan: User user@example.com not found."),
+		status.Error(codes.Unknown, "app/proxyman/command: proxy is not a UserManager"),
+		status.Error(codes.Unknown, "opaque handler response"),
+	} {
+		if !isBenignUserRemovalError(err, absentInbound, "user@example.com") {
+			t.Fatalf("expected benign removal error: %v", err)
+		}
+	}
+
+	presentInbound := &Inbound{clients: map[string]api.Account{
+		"user@example.com": trojanAccount("user@example.com", "secret"),
+	}}
+	for _, err := range []error{
+		status.Error(codes.Unknown, "user not found"),
+		status.Error(codes.Unavailable, "connection refused"),
+		status.Error(codes.DeadlineExceeded, "deadline exceeded"),
+		errors.New("user not found"),
+		errors.New("local handler failure"),
+	} {
+		if isBenignUserRemovalError(err, presentInbound, "user@example.com") {
+			t.Fatalf("unexpectedly accepted runtime failure: %v", err)
+		}
+	}
+}