Skip to content

Commit 63a78af

Browse files
committed
node: Validate and sanitize RPC URLs
- Checks watcher's URLs against a deny-list of public RPC URLs. Logs an error when a public RPC URL is found for a delegated guardian. This should prevent problems where a delegate is not correct pointed at a full node. - Sanitizes URLs in logs so that path parts are not included. (These can sometimes contain API keys or other sensitive strings). - Normalizes logged field names across watchers for the 'watcher started' log
1 parent ccacae1 commit 63a78af

24 files changed

Lines changed: 260 additions & 32 deletions

node/pkg/common/url_verification.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,27 @@ func ValidateURL(urlStr string, validSchemes []string) bool {
4040
}
4141
return false
4242
}
43+
44+
// SafeURLForLogging returns only the hostname for a URL-like string.
45+
// It intentionally omits userinfo, path, query, and fragment because those may contain credentials.
46+
func SafeURLForLogging(urlStr string) string {
47+
if urlStr == "" {
48+
return ""
49+
}
50+
51+
parsedURL, err := url.Parse(urlStr)
52+
if err != nil || parsedURL.Host == "" {
53+
// Schemeless host:port strings parse as scheme:opaque. Parse them again
54+
// as network-path references so URL.Hostname can extract the host.
55+
parsedURL, err = url.Parse("//" + urlStr)
56+
}
57+
if err != nil {
58+
return "<invalid-url>"
59+
}
60+
61+
if host := parsedURL.Hostname(); host != "" {
62+
return host
63+
}
64+
65+
return "<invalid-url>"
66+
}

node/pkg/common/url_verification_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,25 @@ func TestValidateURL(t *testing.T) {
3838
assert.Equal(t, test.expected, result)
3939
}
4040
}
41+
42+
func TestSafeURLForLogging(t *testing.T) {
43+
tests := []struct {
44+
name string
45+
urlStr string
46+
expected string
47+
}{
48+
{name: "https with credentials", urlStr: "https://user:pass@example.com/path?api_key=secret", expected: "example.com"},
49+
{name: "websocket", urlStr: "wss://rpc.example.com/websocket", expected: "rpc.example.com"},
50+
{name: "host port without scheme", urlStr: "example.com:8080/path?token=secret", expected: "example.com"},
51+
{name: "credentials without scheme", urlStr: "user:pass@example.com/path?api_key=secret", expected: "example.com"},
52+
{name: "ipv6", urlStr: "http://[::1]:8545/path", expected: "::1"},
53+
{name: "ipv6 without scheme", urlStr: "[::1]:8545/path", expected: "::1"},
54+
{name: "empty", urlStr: "", expected: ""},
55+
}
56+
57+
for _, test := range tests {
58+
t.Run(test.name, func(t *testing.T) {
59+
assert.Equal(t, test.expected, SafeURLForLogging(test.urlStr))
60+
})
61+
}
62+
}

node/pkg/processor/processor.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"slices"
88
"sort"
9+
"strings"
910
"sync"
1011
"time"
1112

@@ -25,6 +26,7 @@ import (
2526
"github.com/certusone/wormhole/node/pkg/gwrelayer"
2627
gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1"
2728
"github.com/certusone/wormhole/node/pkg/supervisor"
29+
"github.com/certusone/wormhole/node/pkg/watchers"
2830
"github.com/wormhole-foundation/wormhole/sdk/vaa"
2931

3032
"github.com/prometheus/client_golang/prometheus"
@@ -210,6 +212,9 @@ type Processor struct {
210212

211213
// managerC is the channel used to send signed VAAs to the manager service (nil if manager service is disabled)
212214
managerC chan<- *vaa.VAA
215+
216+
// publicRpcLogged tracks which chains have already had a public RPC warning logged.
217+
publicRpcLogged map[vaa.ChainID]struct{}
213218
}
214219

215220
// updateVaaEntry is used to queue up a VAA to be written to the database.
@@ -284,6 +289,101 @@ var (
284289
// batchObsvPubChanSize specifies the size of the channel used to publish observation batches. Allow five seconds worth.
285290
const batchObsvPubChanSize = p2p.MaxObservationBatchSize * 5
286291

292+
// publicRPCDenyList contains domains for known public RPC endpoints.
293+
// Guardians using these for watcher URLs are at risk of serving incorrect data,
294+
// which can lead to consensus issues when the guardian is part of the delegated guardian set.
295+
var publicRPCDenyList = map[string]struct{}{
296+
"infura.io": {},
297+
"alchemy.com": {},
298+
"quiknode.pro": {},
299+
"ankr.com": {},
300+
"blastapi.io": {},
301+
"publicnode.com": {},
302+
"llamarpc.com": {},
303+
"1rpc.io": {},
304+
"nodereal.io": {},
305+
"bnbchain.org": {},
306+
"pocket.network": {},
307+
"monad.xyz": {},
308+
"monadinfra.com": {},
309+
"hyperliquid.xyz": {},
310+
"t.conduit.xyz": {},
311+
}
312+
313+
func publicRPCDomain(host string) (string, bool) {
314+
host = strings.ToLower(strings.TrimSuffix(host, "."))
315+
for domain := host; domain != ""; {
316+
if _, exists := publicRPCDenyList[domain]; exists {
317+
return domain, true
318+
}
319+
320+
_, parentDomain, foundParentDomain := strings.Cut(domain, ".")
321+
if !foundParentDomain {
322+
return "", false
323+
}
324+
domain = parentDomain
325+
}
326+
327+
return "", false
328+
}
329+
330+
// checkPublicRpcEndpoints checks all chains in the DGS for public RPC URLs.
331+
// It logs an error once per chain per run if a public RPC endpoint is detected.
332+
// This is a safety measure: delegating guardians that use public RPC endpoints
333+
// risk producing incorrect VAAs, which can cause consensus failures.
334+
func (p *Processor) checkPublicRpcEndpoints() {
335+
if !p.delegatedGuardiansEnabled {
336+
return
337+
}
338+
if p.dgc == nil {
339+
p.logger.Warn("public RPC endpoint check skipped: delegated guardian config is nil")
340+
return
341+
}
342+
343+
allChains := p.dgc.ReadAll()
344+
if len(allChains) == 0 {
345+
p.logger.Warn("public RPC endpoint check skipped: delegated guardian config has no chains")
346+
return
347+
}
348+
349+
for chainID, cfg := range allChains {
350+
// Only check chains where this guardian is a delegated guardian
351+
if _, ok := cfg.KeyIndex(p.ourAddr); !ok {
352+
continue
353+
}
354+
355+
// Skip if we've already logged for this chain.
356+
if _, logged := p.publicRpcLogged[chainID]; logged {
357+
continue
358+
}
359+
360+
rpcURLs := watchers.RPCURLs(chainID)
361+
if len(rpcURLs) == 0 {
362+
continue
363+
}
364+
365+
for _, rpcURL := range rpcURLs {
366+
hostname := common.SafeURLForLogging(rpcURL)
367+
if hostname == "<invalid-url>" {
368+
continue
369+
}
370+
371+
if domain, ok := publicRPCDomain(hostname); ok {
372+
p.logger.Error("public RPC endpoint detected for delegated guardian chain",
373+
zap.Stringer("chainID", chainID),
374+
zap.String("rpcHost", hostname),
375+
zap.String("matchedDomain", domain),
376+
)
377+
p.publicRpcLogged[chainID] = struct{}{}
378+
}
379+
380+
if _, logged := p.publicRpcLogged[chainID]; logged {
381+
break
382+
}
383+
}
384+
}
385+
}
386+
287387
func NewProcessor(
288388
ctx context.Context,
289389
db *guardianDB.Database,
@@ -345,6 +445,7 @@ func NewProcessor(
345445
dgc: dgc,
346446
delegatedGuardiansEnabled: delegatedGuardiansEnabled,
347447
managerC: managerC,
448+
publicRpcLogged: make(map[vaa.ChainID]struct{}),
348449
}
349450
}
350451

@@ -509,6 +610,8 @@ func (p *Processor) Run(ctx context.Context) error {
509610

510611
if err := p.dgc.Set(chains); err != nil {
511612
p.logger.Error("delegate guardian config update failed", zap.Error(err))
613+
} else {
614+
p.checkPublicRpcEndpoints()
512615
}
513616
dgConfig.mu.Unlock()
514617
case k := <-p.msgC:
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package processor
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestPublicRPCDomain(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
host string
13+
expectedDomain string
14+
expectedMatch bool
15+
}{
16+
{name: "exact match", host: "publicnode.com", expectedDomain: "publicnode.com", expectedMatch: true},
17+
{name: "subdomain match", host: "rpc.monad.xyz", expectedDomain: "monad.xyz", expectedMatch: true},
18+
{name: "nested subdomain match", host: "rpc-plume-mainnet-1.t.conduit.xyz", expectedDomain: "t.conduit.xyz", expectedMatch: true},
19+
{name: "case insensitive", host: "RPC.HYPERLIQUID.XYZ", expectedDomain: "hyperliquid.xyz", expectedMatch: true},
20+
{name: "trailing dot", host: "polygon-mainnet.quiknode.pro.", expectedDomain: "quiknode.pro", expectedMatch: true},
21+
{name: "no match", host: "guardian.example.com", expectedMatch: false},
22+
}
23+
24+
for _, test := range tests {
25+
t.Run(test.name, func(t *testing.T) {
26+
domain, ok := publicRPCDomain(test.host)
27+
assert.Equal(t, test.expectedMatch, ok)
28+
assert.Equal(t, test.expectedDomain, domain)
29+
})
30+
}
31+
}

node/pkg/watchers/algorand/config.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,7 @@ func (wc *WatcherConfig) Create(
3737
_ chan<- *common.GuardianSet,
3838
_ common.Environment,
3939
) (supervisor.Runnable, interfaces.Reobserver, error) {
40+
watchers.RegisterRPCURL(wc.ChainID, wc.IndexerRPC)
41+
watchers.RegisterRPCURL(wc.ChainID, wc.AlgodRPC)
4042
return NewWatcher(wc.IndexerRPC, wc.IndexerToken, wc.AlgodRPC, wc.AlgodToken, wc.AppID, msgC, obsvReqC).Run, nil, nil
4143
}

node/pkg/watchers/algorand/watcher.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,15 +221,13 @@ func (e *Watcher) Run(ctx context.Context) error {
221221

222222
logger.Info("Starting watcher",
223223
zap.String("watcher_name", "algorand"),
224-
zap.String("indexerRPC", e.indexerRPC),
225-
zap.String("indexerToken", e.indexerToken),
226-
zap.String("algodRPC", e.algodRPC),
227-
zap.String("algodToken", e.algodToken),
224+
zap.String("indexerURL", common.SafeURLForLogging(e.indexerRPC)),
225+
zap.String("algodURL", common.SafeURLForLogging(e.algodRPC)),
228226
zap.Uint64("appid", e.appid),
229227
)
230228

231-
logger.Info("Algorand watcher connecting to indexer ", zap.String("url", e.indexerRPC))
232-
logger.Info("Algorand watcher connecting to RPC node ", zap.String("url", e.algodRPC))
229+
logger.Info("Algorand watcher connecting to indexer ", zap.String("indexerURL", common.SafeURLForLogging(e.indexerRPC)))
230+
logger.Info("Algorand watcher connecting to RPC node ", zap.String("algodURL", common.SafeURLForLogging(e.algodRPC)))
233231

234232
timer := time.NewTicker(time.Second * 1)
235233
defer timer.Stop()

node/pkg/watchers/aptos/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ func (wc *WatcherConfig) Create(
3535
_ chan<- *common.GuardianSet,
3636
_ common.Environment,
3737
) (supervisor.Runnable, interfaces.Reobserver, error) {
38+
watchers.RegisterRPCURL(wc.ChainID, wc.Rpc)
3839
return NewWatcher(wc.ChainID, wc.NetworkID, wc.Rpc, wc.Account, wc.Handle, msgC, obsvReqC).Run, nil, nil
3940
}

node/pkg/watchers/aptos/watcher.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (e *Watcher) Run(ctx context.Context) error {
8585

8686
logger.Info("Starting watcher",
8787
zap.String("watcher_name", e.networkID),
88-
zap.String("rpc", e.aptosRPC),
88+
zap.String("rpcURL", common.SafeURLForLogging(e.aptosRPC)),
8989
zap.String("account", e.aptosAccount),
9090
zap.String("handle", e.aptosHandle),
9191
)
@@ -99,9 +99,9 @@ func (e *Watcher) Run(ctx context.Context) error {
9999
var aptosHealth = fmt.Sprintf(`%s/v1`, e.aptosRPC)
100100

101101
logger.Info("watcher connecting to RPC node ",
102-
zap.String("url", e.aptosRPC),
103-
zap.String("eventsQuery", eventsEndpoint),
104-
zap.String("healthQuery", aptosHealth),
102+
zap.String("rpcURL", common.SafeURLForLogging(e.aptosRPC)),
103+
zap.String("eventsURL", common.SafeURLForLogging(eventsEndpoint)),
104+
zap.String("healthURL", common.SafeURLForLogging(aptosHealth)),
105105
)
106106

107107
// the events have sequence numbers associated with them in the aptos API

node/pkg/watchers/cosmwasm/config.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,7 @@ func (wc *WatcherConfig) Create(
3535
_ chan<- *common.GuardianSet,
3636
env common.Environment,
3737
) (supervisor.Runnable, interfaces.Reobserver, error) {
38+
watchers.RegisterRPCURL(wc.ChainID, wc.Websocket)
39+
watchers.RegisterRPCURL(wc.ChainID, wc.Lcd)
3840
return NewWatcher(wc.Websocket, wc.Lcd, wc.Contract, msgC, obsvReqC, wc.ChainID, env).Run, nil, nil
3941
}

node/pkg/watchers/cosmwasm/watcher.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,13 @@ func (e *Watcher) Run(ctx context.Context) error {
153153

154154
logger.Info("Starting watcher",
155155
zap.String("watcher_name", "cosmwasm"),
156-
zap.String("urlWS", e.urlWS),
157-
zap.String("urlLCD", e.urlLCD),
156+
zap.String("wsURL", common.SafeURLForLogging(e.urlWS)),
157+
zap.String("lcdURL", common.SafeURLForLogging(e.urlLCD)),
158158
zap.String("contract", e.contract),
159159
zap.String("chainID", e.chainID.String()),
160160
)
161161

162-
logger.Info("connecting to websocket", zap.String("network", e.networkName), zap.String("url", e.urlWS))
162+
logger.Info("connecting to websocket", zap.String("network", e.networkName), zap.String("wsURL", common.SafeURLForLogging(e.urlWS)))
163163

164164
//nolint:bodyclose // The close is down below. The linter misses it.
165165
c, _, err := websocket.Dial(ctx, e.urlWS, nil)

0 commit comments

Comments
 (0)