fix: sanitize url in http client debug logs and metrics (#6974)

* fix: sanitize url in http client debug logs and metrics

* Improve naming consistency

* Clean up

* Re-assign baseUrl to fix type inference

packages/api/src/utils/client/httpClient.ts

@@ -1,4 +1,4 @@
-import {ErrorAborted, Logger, MapDef, TimeoutError, isValidHttpUrl, retry} from "@lodestar/utils";
+import {ErrorAborted, Logger, MapDef, TimeoutError, isValidHttpUrl, retry, toPrintableUrl} from "@lodestar/utils";
 import {mergeHeaders} from "../headers.js";
 import {Endpoint} from "../types.js";
 import {WireFormat} from "../wireFormat.js";
@@ -115,7 +115,12 @@ export class HttpClient implements IHttpClient {
       }
       // De-duplicate by baseUrl, having two baseUrls with different token or timeouts does not make sense
       if (!this.urlsInits.some((opt) => opt.baseUrl === urlInit.baseUrl)) {
-        this.urlsInits.push({...urlInit, urlIndex: i} as UrlInitRequired);
+        this.urlsInits.push({
+          ...urlInit,
+          baseUrl: urlInit.baseUrl,
+          urlIndex: i,
+          printableUrl: toPrintableUrl(urlInit.baseUrl),
+        });
       }
     }
 
@@ -134,7 +139,7 @@ export class HttpClient implements IHttpClient {
     if (metrics) {
       metrics.urlsScore.addCollect(() => {
         for (let i = 0; i < this.urlsScore.length; i++) {
-          metrics.urlsScore.set({urlIndex: i, baseUrl: this.urlsInits[i].baseUrl}, this.urlsScore[i]);
+          metrics.urlsScore.set({urlIndex: i, baseUrl: this.urlsInits[i].printableUrl}, this.urlsScore[i]);
         }
       });
     }
@@ -185,12 +190,12 @@ export class HttpClient implements IHttpClient {
           // - If url[0] is good, only send to 0
           // - If url[0] has recently errored, send to both 0, 1, etc until url[0] does not error for some time
           for (; i < this.urlsInits.length; i++) {
-            const baseUrl = this.urlsInits[i].baseUrl;
+            const {printableUrl} = this.urlsInits[i];
             const routeId = definition.operationId;
 
             if (i > 0) {
-              this.metrics?.requestToFallbacks.inc({routeId, baseUrl});
-              this.logger?.debug("Requesting fallback URL", {routeId, baseUrl, score: this.urlsScore[i]});
+              this.metrics?.requestToFallbacks.inc({routeId, baseUrl: printableUrl});
+              this.logger?.debug("Requesting fallback URL", {routeId, baseUrl: printableUrl, score: this.urlsScore[i]});
             }
 
             // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -217,7 +222,11 @@ export class HttpClient implements IHttpClient {
                   if (++errorCount >= requestCount) {
                     resolve(res);
                   } else {
-                    this.logger?.debug("Request error, retrying", {routeId, baseUrl}, res.error() as Error);
+                    this.logger?.debug(
+                      "Request error, retrying",
+                      {routeId, baseUrl: printableUrl},
+                      res.error() as Error
+                    );
                   }
                 }
               },
@@ -229,7 +238,7 @@ export class HttpClient implements IHttpClient {
                 if (++errorCount >= requestCount) {
                   reject(err);
                 } else {
-                  this.logger?.debug("Request error, retrying", {routeId, baseUrl}, err);
+                  this.logger?.debug("Request error, retrying", {routeId, baseUrl: printableUrl}, err);
                 }
               }
             );
@@ -347,7 +356,7 @@ export class HttpClient implements IHttpClient {
     abortSignals.forEach((s) => s?.addEventListener("abort", onSignalAbort));
 
     const routeId = definition.operationId;
-    const {baseUrl, requestWireFormat, responseWireFormat} = init;
+    const {printableUrl, requestWireFormat, responseWireFormat} = init;
     const timer = this.metrics?.requestTime.startTimer({routeId});
 
     try {
@@ -359,7 +368,7 @@ export class HttpClient implements IHttpClient {
       if (!apiResponse.ok) {
         await apiResponse.errorBody();
         this.logger?.debug("API response error", {routeId, status: apiResponse.status});
-        this.metrics?.requestErrors.inc({routeId, baseUrl});
+        this.metrics?.requestErrors.inc({routeId, baseUrl: printableUrl});
         return apiResponse;
       }
 
@@ -376,7 +385,7 @@ export class HttpClient implements IHttpClient {
         streamTimer?.();
       }
     } catch (e) {
-      this.metrics?.requestErrors.inc({routeId, baseUrl});
+      this.metrics?.requestErrors.inc({routeId, baseUrl: printableUrl});
 
       if (isAbortedError(e)) {
         if (abortSignals.some((s) => s?.aborted)) {

packages/api/src/utils/client/request.ts

@@ -28,7 +28,12 @@ export type OptionalRequestInit = {
 };
 
 export type UrlInit = ApiRequestInit & {baseUrl?: string};
-export type UrlInitRequired = ApiRequestInit & {urlIndex: number; baseUrl: string};
+export type UrlInitRequired = ApiRequestInit & {
+  urlIndex: number;
+  baseUrl: string;
+  /** Used in logs and metrics to prevent leaking user credentials */
+  printableUrl: string;
+};
 export type ApiRequestInit = ExtraRequestInit & OptionalRequestInit & RequestInit;
 export type ApiRequestInitRequired = Required<ExtraRequestInit> & UrlInitRequired;
 

packages/beacon-node/src/eth1/provider/eth1Provider.ts

@@ -1,7 +1,7 @@
 import {toHexString} from "@chainsafe/ssz";
 import {phase0} from "@lodestar/types";
 import {ChainConfig} from "@lodestar/config";
-import {fromHex, isErrorAborted, createElapsedTimeTracker, toSafePrintableUrl} from "@lodestar/utils";
+import {fromHex, isErrorAborted, createElapsedTimeTracker, toPrintableUrl} from "@lodestar/utils";
 import {Logger} from "@lodestar/logger";
 
 import {FetchError, isFetchError} from "@lodestar/api";
@@ -84,7 +84,7 @@ export class Eth1Provider implements IEth1Provider {
       jwtVersion: opts.jwtVersion,
       metrics: metrics,
     });
-    this.logger?.info("Eth1 provider", {urls: providerUrls.map(toSafePrintableUrl).toString()});
+    this.logger?.info("Eth1 provider", {urls: providerUrls.map(toPrintableUrl).toString()});
 
     this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => {
       const oldState = this.state;

packages/beacon-node/src/execution/builder/http.ts

@@ -14,7 +14,7 @@ import {ChainForkConfig} from "@lodestar/config";
 import {Logger} from "@lodestar/logger";
 import {getClient, ApiClient as BuilderApi} from "@lodestar/api/builder";
 import {SLOTS_PER_EPOCH, ForkExecution} from "@lodestar/params";
-import {toSafePrintableUrl} from "@lodestar/utils";
+import {toPrintableUrl} from "@lodestar/utils";
 import {Metrics} from "../../metrics/metrics.js";
 import {IExecutionBuilder} from "./interface.js";
 
@@ -64,7 +64,7 @@ export class ExecutionBuilderHttp implements IExecutionBuilder {
       },
       {config, metrics: metrics?.builderHttpClient}
     );
-    logger?.info("External builder", {url: toSafePrintableUrl(baseUrl)});
+    logger?.info("External builder", {url: toPrintableUrl(baseUrl)});
     this.config = config;
     this.issueLocalFcUWithFeeRecipient = opts.issueLocalFcUWithFeeRecipient;
 

packages/beacon-node/src/execution/engine/index.ts

@@ -1,4 +1,4 @@
-import {fromHex, toSafePrintableUrl} from "@lodestar/utils";
+import {fromHex, toPrintableUrl} from "@lodestar/utils";
 import {JsonRpcHttpClient} from "../../eth1/provider/jsonRpcHttpClient.js";
 import {IExecutionEngine} from "./interface.js";
 import {ExecutionEngineDisabled} from "./disabled.js";
@@ -39,7 +39,7 @@ export function getExecutionEngineHttp(
     jwtId: opts.jwtId,
     jwtVersion: opts.jwtVersion,
   });
-  modules.logger.info("Execution client", {urls: opts.urls.map(toSafePrintableUrl).toString()});
+  modules.logger.info("Execution client", {urls: opts.urls.map(toPrintableUrl).toString()});
   return new ExecutionEngineHttp(rpc, modules);
 }
 

packages/cli/src/cmds/validator/signers/logSigners.ts

@@ -1,5 +1,5 @@
 import {Signer, SignerLocal, SignerRemote, SignerType} from "@lodestar/validator";
-import {LogLevel, Logger, toSafePrintableUrl} from "@lodestar/utils";
+import {LogLevel, Logger, toPrintableUrl} from "@lodestar/utils";
 import {YargsError} from "../../../util/errors.js";
 import {IValidatorCliArgs} from "../options.js";
 
@@ -29,7 +29,7 @@ export function logSigners(logger: Pick<Logger, LogLevel.info>, signers: Signer[
   }
 
   for (const {url, pubkeys} of groupRemoteSignersByUrl(remoteSigners)) {
-    logger.info(`Remote signers on URL: ${toSafePrintableUrl(url)}`);
+    logger.info(`Remote signers on URL: ${toPrintableUrl(url)}`);
     for (const pubkey of pubkeys) {
       logger.info(pubkey);
     }

packages/utils/src/url.ts

@@ -20,10 +20,10 @@ export function isValidHttpUrl(urlStr: string): boolean {
 }
 
 /**
- * Sanitize URL to prevent leaking user credentials in logs
+ * Sanitize URL to prevent leaking user credentials in logs or metrics
  *
  * Note: `urlStr` must be a valid URL
  */
-export function toSafePrintableUrl(urlStr: string): string {
+export function toPrintableUrl(urlStr: string): string {
   return new URL(urlStr).origin;
 }

packages/validator/src/services/externalSignerSync.ts

@@ -2,7 +2,7 @@ import {fromHexString} from "@chainsafe/ssz";
 import {PublicKey} from "@chainsafe/blst";
 import {ChainForkConfig} from "@lodestar/config";
 import {SLOTS_PER_EPOCH} from "@lodestar/params";
-import {toSafePrintableUrl} from "@lodestar/utils";
+import {toPrintableUrl} from "@lodestar/utils";
 
 import {LoggerVc} from "../util/index.js";
 import {externalSignerGetKeys} from "../util/externalSignerClient.js";
@@ -35,7 +35,7 @@ export function pollExternalSignerPubkeys(
   async function fetchExternalSignerPubkeys(): Promise<void> {
     // External signer URL is already validated earlier
     const externalSignerUrl = externalSigner.url as string;
-    const printableUrl = toSafePrintableUrl(externalSignerUrl);
+    const printableUrl = toPrintableUrl(externalSignerUrl);
 
     try {
       logger.debug("Fetching public keys from external signer", {url: printableUrl});

packages/validator/src/validator.ts

@@ -2,7 +2,7 @@ import {toHexString} from "@chainsafe/ssz";
 import {BLSPubkey, phase0, ssz} from "@lodestar/types";
 import {createBeaconConfig, BeaconConfig, ChainForkConfig} from "@lodestar/config";
 import {Genesis} from "@lodestar/types/phase0";
-import {Logger, toSafePrintableUrl} from "@lodestar/utils";
+import {Logger, toPrintableUrl} from "@lodestar/utils";
 import {getClient, ApiClient, routes, ApiRequestInit, defaultInit} from "@lodestar/api";
 import {computeEpochAtSlot, getCurrentSlot} from "@lodestar/state-transition";
 import {Clock, IClock} from "./util/clock.js";
@@ -132,7 +132,7 @@ export class Validator {
       // not be any errors in the logs due to fallback nodes handling the requests
       const {httpClient} = this.api;
       if (httpClient.urlsInits.length > 1) {
-        const primaryNodeUrl = toSafePrintableUrl(httpClient.urlsInits[0].baseUrl);
+        const primaryNodeUrl = toPrintableUrl(httpClient.urlsInits[0].baseUrl);
 
         this.clock.runEveryEpoch(async () => {
           // Only emit warning if URL score is 0 to prevent false positives
@@ -293,7 +293,7 @@ export class Validator {
       // not necessary since this instance won't be used for validator duties
       api = getClient({urls, globalInit: {signal: opts.abortController.signal, ...globalInit}}, {config, logger});
       logger.info("Beacon node", {
-        urls: urls.map(toSafePrintableUrl).toString(),
+        urls: urls.map(toPrintableUrl).toString(),
         requestWireFormat: globalInit?.requestWireFormat ?? defaultInit.requestWireFormat,
         responseWireFormat: globalInit?.responseWireFormat ?? defaultInit.responseWireFormat,
       });