node: Validate and sanitize RPC URLs

.gitignore

@@ -31,6 +31,7 @@ sui.log.*
 sui/examples/wrapped_coin
 *.prof
 /coverage-check
+scripts/coverage-check/coverage-check
 # Only used for internal testing
 *.testnet.yaml
 coverage.txt

.golangci.yml

@@ -237,6 +237,10 @@ linters:
           - nolintlint
           - prealloc
         path: _test\.go
+      - linters:
+          - gosec
+        path: _test\.go
+        text: 'G101'
       - linters:
           - exhaustruct
         path: (?i).*mock.*

node/cmd/guardiand/node.go

@@ -1200,7 +1200,7 @@ func runNode(cmd *cobra.Command, args []string) {
 		}
 
 		// Connect to wormchain for the accountant.
-		logger.Info("Connecting to wormchain for accountant", zap.String("wormchainURL", *wormchainURL), zap.String("keyPath", keyPathName), zap.String("component", "gacct"))
+		logger.Info("Connecting to wormchain for accountant", zap.String("wormchainURL", common.SafeURLForLogging(*wormchainURL)), zap.String("keyPath", keyPathName), zap.String("component", "gacct"))
 		accountantWormchainConn, err = wormconn.NewConn(*wormchainURL, wormchainKey, wormchainId)
 		if err != nil {
 			logger.Fatal("failed to connect to wormchain for accountant", zap.Error(err), zap.String("component", "gacct"))
@@ -1236,7 +1236,7 @@ func runNode(cmd *cobra.Command, args []string) {
 		}
 
 		// Connect to wormchain for the NTT accountant.
-		logger.Info("Connecting to wormchain for NTT accountant", zap.String("wormchainURL", *wormchainURL), zap.String("keyPath", keyPathName), zap.String("component", "gacct"))
+		logger.Info("Connecting to wormchain for NTT accountant", zap.String("wormchainURL", common.SafeURLForLogging(*wormchainURL)), zap.String("keyPath", keyPathName), zap.String("component", "gacct"))
 		accountantNttWormchainConn, err = wormconn.NewConn(*wormchainURL, wormchainKey, wormchainId)
 		if err != nil {
 			logger.Fatal("failed to connect to wormchain for NTT accountant", zap.Error(err), zap.String("component", "gacct"))
@@ -1270,7 +1270,7 @@ func runNode(cmd *cobra.Command, args []string) {
 			logger.Fatal("failed to load private key", zap.Error(err), zap.String("component", "gwrelayer"))
 		}
 
-		logger.Info("Connecting to wormchain", zap.String("wormchainURL", *wormchainURL), zap.String("keyPath", wormchainKeyPathName), zap.String("component", "gwrelayer"))
+		logger.Info("Connecting to wormchain", zap.String("wormchainURL", common.SafeURLForLogging(*wormchainURL)), zap.String("keyPath", wormchainKeyPathName), zap.String("component", "gwrelayer"))
 		gatewayRelayerWormchainConn, err = wormconn.NewConn(*wormchainURL, wormchainKey, wormchainId)
 		if err != nil {
 			logger.Fatal("failed to connect to wormchain", zap.Error(err), zap.String("component", "gwrelayer"))

node/cmd/txverifier/evm.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 
+	nodecommon "github.com/certusone/wormhole/node/pkg/common"
 	"github.com/certusone/wormhole/node/pkg/telemetry"
 	txverifier "github.com/certusone/wormhole/node/pkg/txverifier"
 	"github.com/certusone/wormhole/node/pkg/version"
@@ -117,7 +118,7 @@ func runTransferVerifierEvm(cmd *cobra.Command, args []string) {
 
 	logger.Info("Starting EVM transfer verifier")
 
-	logger.Debug("EVM rpc connection", zap.String("url", *evmRpc))
+	logger.Debug("EVM rpc connection", zap.String("url", nodecommon.SafeURLForLogging(*evmRpc)))
 	logger.Debug("EVM core contract", zap.String("address", *evmCoreContract))
 	logger.Debug("EVM token bridge contract", zap.String("address", *evmTokenBridgeContract))
 	logger.Debug("EVM wrapped native asset contract", zap.String("address", *wrappedNativeContract))

node/cmd/txverifier/sui.go

@@ -124,14 +124,14 @@ func runTransferVerifierSui(cmd *cobra.Command, args []string) {
 	// Verify CLI parameters
 	if *suiRPC == "" || *suiCoreBridgePackageId == "" || *suiTokenBridgeEmitter == "" || *suiTokenBridgePackageId == "" {
 		logger.Fatal("One or more CLI parameters are empty",
-			zap.String("suiRPC", *suiRPC),
+			zap.String("suiRPC", common.SafeURLForLogging(*suiRPC)),
 			zap.String("suiCoreBridgePackageId", *suiCoreBridgePackageId),
 			zap.String("suiTokenBridgeEmitter", *suiTokenBridgeEmitter),
 			zap.String("suiTokenBridgePackageId", *suiTokenBridgePackageId))
 	}
 
 	logger.Info("Starting Sui transfer verifier")
-	logger.Debug("Sui rpc connection", zap.String("url", *suiRPC))
+	logger.Debug("Sui rpc connection", zap.String("url", common.SafeURLForLogging(*suiRPC)))
 	logger.Debug("Sui core bridge package ID", zap.String("packageId", *suiCoreBridgePackageId))
 	logger.Debug("Sui token bridge package ID", zap.String("packageId", *suiTokenBridgePackageId))
 	logger.Debug("Sui token bridge emitter", zap.String("address", *suiTokenBridgeEmitter))

node/pkg/accountant/watcher.go

@@ -39,16 +39,16 @@ func (acct *Accountant) watcher(ctx context.Context, isNTT bool) error {
 	}
 	errC := make(chan error)
 
-	acct.logger.Info(fmt.Sprintf("acctwatch: creating %s watcher", tag), zap.String("url", acct.wsUrl), zap.String("contract", contract))
+	acct.logger.Info(fmt.Sprintf("acctwatch: creating %s watcher", tag), zap.String("url", common.SafeURLForLogging(acct.wsUrl)), zap.String("contract", contract))
 	tmConn, err := tmHttp.New(acct.wsUrl, "/websocket")
 	if err != nil {
 		connectionErrors.Inc()
-		return fmt.Errorf("failed to establish %s tendermint connection: %w", tag, err)
+		return fmt.Errorf("failed to establish %s tendermint connection: %s", tag, common.SafeErrorForLogging(err, acct.wsUrl))
 	}
 
 	if startErr := tmConn.Start(); startErr != nil {
 		connectionErrors.Inc()
-		return fmt.Errorf("failed to start %s tendermint connection: %w", tag, startErr)
+		return fmt.Errorf("failed to start %s tendermint connection: %s", tag, common.SafeErrorForLogging(startErr, acct.wsUrl))
 	}
 	defer func() {
 		if stopErr := tmConn.Stop(); stopErr != nil {

node/pkg/adminrpc/adminserver.go

@@ -1101,21 +1101,22 @@ func (s *nodePrivilegedService) fetchMissing(
 	defer cancel()
 
 	for _, node := range nodes {
-		req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf(
-			"%s/v1/signed_vaa/%d/%s/%d", node, chain, addr, seq), nil)
+		requestURL := fmt.Sprintf("%s/v1/signed_vaa/%d/%s/%d", node, chain, addr, seq)
+		req, err := http.NewRequestWithContext(ctx, "GET", requestURL, nil)
 		if err != nil {
-			return false, fmt.Errorf("failed to create request: %w", err)
+			return false, fmt.Errorf("failed to create request: %s", common.SafeErrorForLogging(err, requestURL))
 		}
 
 		// #nosec G704 -- Admin RPC: BackfillNodes from authorized admin request
 		resp, err := c.Do(req)
 		if err != nil {
+			safeErr := common.SafeErrorForLogging(err, requestURL)
 			s.logger.Warn("failed to fetch missing VAA",
-				zap.String("node", node),
+				zap.String("node", common.SafeURLForLogging(node)),
 				zap.String("chain", chain.String()),
 				zap.String("address", addr),
 				zap.Uint64("sequence", seq),
-				zap.Error(err),
+				zap.String("error", safeErr),
 			)
 			continue
 		}
@@ -1132,7 +1133,7 @@ func (s *nodePrivilegedService) fetchMissing(
 			if err := json.NewDecoder(resp.Body).Decode(&respBody); err != nil {
 				resp.Body.Close()
 				s.logger.Warn("failed to decode VAA response",
-					zap.String("node", node),
+					zap.String("node", common.SafeURLForLogging(node)),
 					zap.String("chain", chain.String()),
 					zap.String("address", addr),
 					zap.Uint64("sequence", seq),
@@ -1146,7 +1147,7 @@ func (s *nodePrivilegedService) fetchMissing(
 			if err != nil {
 				resp.Body.Close()
 				s.logger.Warn("failed to decode VAA body",
-					zap.String("node", node),
+					zap.String("node", common.SafeURLForLogging(node)),
 					zap.String("chain", chain.String()),
 					zap.String("address", addr),
 					zap.Uint64("sequence", seq),
@@ -1674,8 +1675,9 @@ func (s *nodePrivilegedService) GetAndObserveMissingVAAs(ctx context.Context, re
 	// Create the actual request
 	httpRequest, err := http.NewRequestWithContext(ctx, http.MethodPost, url, jsonBodyReader)
 	if err != nil {
-		fmt.Printf("GetAndObserveMissingVAAs: could not create request: %s\n", err)
-		return nil, err
+		safeErr := common.SafeErrorForLogging(err, url)
+		fmt.Printf("GetAndObserveMissingVAAs: could not create request: %s\n", safeErr)
+		return nil, fmt.Errorf("could not create request: %s", safeErr)
 	}
 
 	httpRequest.Header.Set("Content-Type", "application/json")
@@ -1687,8 +1689,9 @@ func (s *nodePrivilegedService) GetAndObserveMissingVAAs(ctx context.Context, re
 	// #nosec G704 -- Admin RPC: URL from authorized admin request
 	results, err := client.Do(httpRequest)
 	if err != nil {
-		fmt.Printf("GetAndObserveMissingVAAs: error making http request: %s\n", err)
-		return nil, err
+		safeErr := common.SafeErrorForLogging(err, url)
+		fmt.Printf("GetAndObserveMissingVAAs: error making http request: %s\n", safeErr)
+		return nil, fmt.Errorf("error making http request: %s", safeErr)
 	}
 
 	defer results.Body.Close()

node/pkg/altpub/alternate_pub.go

@@ -236,7 +236,7 @@ func NewAlternatePublisher(logger *zap.Logger, guardianAddr []byte, configs []st
 		}
 
 		if _, exists := labels[ep.label]; exists {
-			return nil, fmt.Errorf("duplicate label in --additionalPublishEndpoint '%s'", config)
+			return nil, fmt.Errorf("duplicate label in --additionalPublishEndpoint '%s'", sanitizeAdditionalPublishEndpointConfig(config))
 		}
 
 		labels[ep.label] = struct{}{}
@@ -264,31 +264,32 @@ func NewAlternatePublisher(logger *zap.Logger, guardianAddr []byte, configs []st
 
 // parseEndpoint parses an `--additionalPublishEndpoint` parameter into an Endpoint object. It returns an error if the string is invalid.
 func parseEndpoint(config string) (*Endpoint, error) {
+	safeConfig := sanitizeAdditionalPublishEndpointConfig(config)
 	fields := strings.Split(config, ";")
 	if len(fields) < 2 {
-		return nil, fmt.Errorf("not enough fields in --additionalPublishEndpoint '%s': should be at least 2, there are %d", config, len(fields))
+		return nil, fmt.Errorf("not enough fields in --additionalPublishEndpoint '%s': should be at least 2, there are %d", safeConfig, len(fields))
 	}
 
 	if len(fields) > 4 {
-		return nil, fmt.Errorf("too many fields in --additionalPublishEndpoint '%s': may not be more than 4, there are %d", config, len(fields))
+		return nil, fmt.Errorf("too many fields in --additionalPublishEndpoint '%s': may not be more than 4, there are %d", safeConfig, len(fields))
 	}
 
 	label := fields[0]
 	if len(label) == 0 {
-		return nil, fmt.Errorf("invalid label in --additionalPublishEndpoint '%s': may not be zero length", config)
+		return nil, fmt.Errorf("invalid label in --additionalPublishEndpoint '%s': may not be zero length", safeConfig)
 	}
 
 	baseUrl := fields[1]
 	if valid := common.ValidateURL(baseUrl, []string{"http", "https"}); !valid {
-		return nil, fmt.Errorf("invalid url in --additionalPublishEndpoint '%s': must be `http` or `https`", config)
+		return nil, fmt.Errorf("invalid url in --additionalPublishEndpoint '%s': must be `http` or `https`", safeConfig)
 	}
 
 	delay := time.Duration(0)
 	if len(fields) > 2 && len(fields[2]) != 0 {
 		var err error
 		delay, err = time.ParseDuration(fields[2])
 		if err != nil {
-			return nil, fmt.Errorf("invalid delay duration in --additionalPublishEndpoint '%s', delay %s: %w", config, fields[2], err)
+			return nil, fmt.Errorf("invalid delay duration in --additionalPublishEndpoint '%s', delay %s: %w", safeConfig, fields[2], err)
 		}
 	}
 
@@ -304,7 +305,7 @@ func parseEndpoint(config string) (*Endpoint, error) {
 		for _, str := range chainIds {
 			chainId, err := vaa.StringToKnownChainID(str)
 			if err != nil {
-				return nil, fmt.Errorf("invalid chain ID --additionalPublishEndpoint '%s' ('%s'): %w", config, str, err)
+				return nil, fmt.Errorf("invalid chain ID --additionalPublishEndpoint '%s' ('%s'): %w", safeConfig, str, err)
 			}
 
 			enabledChainsMap[chainId] = struct{}{}
@@ -332,6 +333,16 @@ func parseEndpoint(config string) (*Endpoint, error) {
 	return ep, nil
 }
 
+func sanitizeAdditionalPublishEndpointConfig(config string) string {
+	fields := strings.Split(config, ";")
+	if len(fields) < 2 || fields[1] == "" {
+		return config
+	}
+
+	fields[1] = common.SafeURLForLogging(fields[1])
+	return strings.Join(fields, ";")
+}
+
 // GetFeatures returns the status string to be published in P2P heartbeats. For now, it just returns a static string
 // listing the enabled endpoints, but in the future, it might return the actual status of each endpoint or something.
 // NOTE: `node.getStaticFeatureFlags` assumes that this does not change after initialization.
@@ -353,7 +364,7 @@ func (ap *AlternatePublisher) Run(ctx context.Context) error {
 	ap.startHttpWorkers(ctx, client, errC)
 
 	for _, ep := range ap.endpoints {
-		ap.logger.Info("Enabling endpoint", zap.String("endpoint", ep.label), zap.String("url", ep.baseUrl), zap.Stringer("delay", ep.delay), zap.Stringer("enabledChains", ep.enabledChains))
+		ap.logger.Info("Enabling endpoint", zap.String("endpoint", ep.label), zap.String("url", common.SafeURLForLogging(ep.baseUrl)), zap.Stringer("delay", ep.delay), zap.Stringer("enabledChains", ep.enabledChains))
 		if ep.delay != 0 {
 			worker := &BatchWorker{ap, ep}
 			common.RunWithScissors(ctx, errC, fmt.Sprintf("alt_pub_batcher_%s", ep.label), worker.batchWorker)
@@ -464,7 +475,7 @@ func (ap *AlternatePublisher) httpPost(ctx context.Context, client *http.Client,
 	r, err := http.NewRequestWithContext(ctx, "POST", req.url, bytes.NewBuffer(bytes.Clone(req.data)))
 	if err != nil {
 		requestFailed.WithLabelValues(req.ep.label, "create_failed").Inc()
-		return fmt.Errorf("create failed: %w", err)
+		return fmt.Errorf("create failed: %s", common.SafeErrorForLogging(err, req.url))
 	}
 	r.Header.Add("Content-Type", "application/octet-stream")
 
@@ -477,7 +488,7 @@ func (ap *AlternatePublisher) httpPost(ctx context.Context, client *http.Client,
 		} else {
 			requestFailed.WithLabelValues(req.ep.label, "post_failed").Inc()
 		}
-		return fmt.Errorf("post failed: %w", err)
+		return fmt.Errorf("post failed: %s", common.SafeErrorForLogging(err, req.url))
 	}
 	stop := time.Now()
 	resp.Body.Close()

node/pkg/common/url_verification.go

@@ -1,6 +1,7 @@
 package common
 
 import (
+	"errors"
 	"net"
 	"net/url"
 	"strings"
@@ -40,3 +41,70 @@ func ValidateURL(urlStr string, validSchemes []string) bool {
 	}
 	return false
 }
+
+// SafeURLForLogging returns only the host and port for a URL-like string.
+// It intentionally omits userinfo, path, query, and fragment because those may contain credentials.
+func SafeURLForLogging(urlStr string) string {
+	if urlStr == "" {
+		return ""
+	}
+
+	parsedURL, err := url.Parse(urlStr)
+	if err != nil || parsedURL.Host == "" {
+		// Schemeless host:port strings parse as scheme:opaque. Parse them again
+		// as network-path references so URL.Hostname can extract the host.
+		parsedURL, err = url.Parse("//" + urlStr)
+	}
+	if err != nil {
+		return "<invalid-url>"
+	}
+
+	if host := parsedURL.Host; host != "" {
+		return host
+	}
+
+	return "<invalid-url>"
+}
+
+// SafeErrorForLogging returns an error string with a raw URL replaced by its safe logging form.
+func SafeErrorForLogging(err error, urlStr string) string {
+	if err == nil {
+		return ""
+	}
+	if urlStr == "" {
+		return err.Error()
+	}
+
+	safeURL := SafeURLForLogging(urlStr)
+	errStr := err.Error()
+
+	var urlErr *url.Error
+	if errors.As(err, &urlErr) && urlErr.URL != "" {
+		// url.Error carries the URL separately. Sanitize that exact value in the
+		// full error string so wrapped errors keep their additional context.
+		errStr = strings.ReplaceAll(errStr, urlErr.URL, SafeURLForLogging(urlErr.URL))
+	}
+
+	errStr = strings.ReplaceAll(errStr, urlStr, safeURL)
+
+	parsedURL, parseErr := url.Parse(urlStr)
+	if parseErr != nil {
+		return errStr
+	}
+
+	// Some libraries report URLs through url.URL.String(), which can escape or otherwise
+	// canonicalize the original string.
+	canonicalURL := parsedURL.String()
+	errStr = strings.ReplaceAll(errStr, canonicalURL, safeURL)
+
+	if parsedURL.User != nil {
+		if _, hasPassword := parsedURL.User.Password(); hasPassword {
+			// net/http redacts userinfo passwords as "***" in *url.Error while leaving
+			// path and query values intact, so the raw URL no longer matches exactly.
+			redactedURL := strings.Replace(canonicalURL, parsedURL.User.String()+"@", parsedURL.User.Username()+":***@", 1)
+			errStr = strings.ReplaceAll(errStr, redactedURL, safeURL)
+		}
+	}
+
+	return errStr
+}