From d64cdca951bce6a2c2cae4b8570c29e6cfba2d45 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 11 Dec 2023 21:52:03 +0100 Subject: [PATCH] Ensure URLs are logged after validation --- .../src/eth1/provider/eth1Provider.ts | 2 +- .../beacon-node/src/execution/builder/http.ts | 2 +- .../beacon-node/src/execution/engine/index.ts | 2 +- packages/utils/src/url.ts | 9 +++---- packages/utils/test/unit/url.test.ts | 25 ------------------- packages/validator/src/validator.ts | 2 +- 6 files changed, 7 insertions(+), 35 deletions(-) delete mode 100644 packages/utils/test/unit/url.test.ts diff --git a/packages/beacon-node/src/eth1/provider/eth1Provider.ts b/packages/beacon-node/src/eth1/provider/eth1Provider.ts index 55828582532c..d594c74a3abc 100644 --- a/packages/beacon-node/src/eth1/provider/eth1Provider.ts +++ b/packages/beacon-node/src/eth1/provider/eth1Provider.ts @@ -75,7 +75,6 @@ export class Eth1Provider implements IEth1Provider { this.depositContractAddress = toHexString(config.DEPOSIT_CONTRACT_ADDRESS); const providerUrls = opts.providerUrls ?? DEFAULT_PROVIDER_URLS; - this.logger?.info("Eth1 provider", {urls: providerUrls.map(toSafePrintableUrl).toString()}); this.rpc = new JsonRpcHttpClient(providerUrls, { signal, // Don't fallback with is truncated error. Throw early and let the retry on this class handle it @@ -85,6 +84,7 @@ export class Eth1Provider implements IEth1Provider { jwtVersion: opts.jwtVersion, metrics: metrics, }); + this.logger?.info("Eth1 provider", {urls: providerUrls.map(toSafePrintableUrl).toString()}); this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { const oldState = this.state; diff --git a/packages/beacon-node/src/execution/builder/http.ts b/packages/beacon-node/src/execution/builder/http.ts index 9c80a96f79a5..7b78bfbd31a1 100644 --- a/packages/beacon-node/src/execution/builder/http.ts +++ b/packages/beacon-node/src/execution/builder/http.ts @@ -50,7 +50,6 @@ export class ExecutionBuilderHttp implements IExecutionBuilder { ) { const baseUrl = opts.urls[0]; if (!baseUrl) throw Error("No Url provided for executionBuilder"); - logger?.info("External builder", {url: toSafePrintableUrl(baseUrl)}); this.api = getClient( { baseUrl, @@ -59,6 +58,7 @@ export class ExecutionBuilderHttp implements IExecutionBuilder { }, {config, metrics: metrics?.builderHttpClient} ); + logger?.info("External builder", {url: toSafePrintableUrl(baseUrl)}); this.config = config; this.issueLocalFcUWithFeeRecipient = opts.issueLocalFcUWithFeeRecipient; diff --git a/packages/beacon-node/src/execution/engine/index.ts b/packages/beacon-node/src/execution/engine/index.ts index ec4ad48d73d0..2d92a439c86d 100644 --- a/packages/beacon-node/src/execution/engine/index.ts +++ b/packages/beacon-node/src/execution/engine/index.ts @@ -31,7 +31,6 @@ export function getExecutionEngineHttp( opts: ExecutionEngineHttpOpts, modules: ExecutionEngineModules ): IExecutionEngine { - modules.logger.info("Execution client", {urls: opts.urls.map(toSafePrintableUrl).toString()}); const rpc = new JsonRpcHttpClient(opts.urls, { ...opts, signal: modules.signal, @@ -40,6 +39,7 @@ export function getExecutionEngineHttp( jwtId: opts.jwtId, jwtVersion: opts.jwtVersion, }); + modules.logger.info("Execution client", {urls: opts.urls.map(toSafePrintableUrl).toString()}); return new ExecutionEngineHttp(rpc, modules); } diff --git a/packages/utils/src/url.ts b/packages/utils/src/url.ts index 0f55994e69fd..7d0b23347617 100644 --- a/packages/utils/src/url.ts +++ b/packages/utils/src/url.ts @@ -21,12 +21,9 @@ export function isValidHttpUrl(urlStr: string): boolean { /** * Sanitize URL to prevent leaking user credentials in logs + * + * Note: `urlStr` must be a valid URL */ export function toSafePrintableUrl(urlStr: string): string { - try { - return new URL(urlStr).origin; - } catch (_) { - // Best effort to sanitize value if an invalid URL is provided - return urlStr.replace(/(.*?:\/\/|.*?:\/)?(.*?:.*?@)/, "$1"); - } + return new URL(urlStr).origin; } diff --git a/packages/utils/test/unit/url.test.ts b/packages/utils/test/unit/url.test.ts deleted file mode 100644 index 0dd744a0b297..000000000000 --- a/packages/utils/test/unit/url.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import "../setup.js"; -import {expect} from "chai"; -import {toSafePrintableUrl} from "../../src/url.js"; - -describe("toSafePrintableUrl", () => { - const testCases = [ - {input: "http://user:password@localhost", output: "http://localhost"}, - {input: "http://localhost?apikey=secret", output: "http://localhost"}, - {input: "http://@localhost", output: "http://localhost"}, - {input: "http://:password@localhost", output: "http://localhost"}, - {input: "http://user%3Apassword@localhost", output: "http://localhost"}, - {input: "http:user@localhost", output: "http://localhost"}, - {input: "http://#user:password@localhost", output: "http://localhost"}, - {input: "http/:/user:password@localhost", output: "http/:/localhost"}, - {input: "://user:password@localhost", output: "://localhost"}, - {input: "http/user:password@localhost", output: "localhost"}, - {input: "invalid url", output: "invalid url"}, - ]; - - for (const {input, output} of testCases) { - it(`should sanitize URL ${input}`, () => { - expect(toSafePrintableUrl(input)).to.be.equal(output); - }); - } -}); diff --git a/packages/validator/src/validator.ts b/packages/validator/src/validator.ts index 4a23cbd679c1..571aec71e019 100644 --- a/packages/validator/src/validator.ts +++ b/packages/validator/src/validator.ts @@ -268,10 +268,10 @@ export class Validator { let api: Api; if (typeof opts.api === "string" || Array.isArray(opts.api)) { const urls = typeof opts.api === "string" ? [opts.api] : opts.api; - logger.info("Beacon node", {urls: urls.map(toSafePrintableUrl).toString()}); // This new api instance can make do with default timeout as a faster timeout is // not necessary since this instance won't be used for validator duties api = getClient({urls, getAbortSignal: () => opts.abortController.signal}, {config, logger}); + logger.info("Beacon node", {urls: urls.map(toSafePrintableUrl).toString()}); } else { api = opts.api; }