diff --git a/README.md b/README.md index 9c4364e..be4e0a6 100644 --- a/README.md +++ b/README.md @@ -2168,6 +2168,17 @@ the log level and turns the banner on, and that is all it may ever do: it feeds constant directly, so no runtime toggle in a release build can reach the hardcoded test phrase. +At the raised log level the console also shows the wallet's addresses with their +balances and ENS names, the token contracts looked up, and a line for each +request made through `debugFetch` in `src/shared/log.js` (the explorer, the +price feed, the RPC calls a site makes, and the endpoint checks in settings) and +for its response. A request is logged by its HTTP method, the origin of its URL +(scheme, host and port) and, for a JSON-RPC call, the method name; the balance +refresh, the token lookup and a failed endpoint check in settings name the +endpoint by its origin too. The URL's path and query string, where RPC providers +put API keys, any user name and password in it, and the request body are never +logged. + ### Key Decisions - **No framework**: The popup UI is vanilla JS and HTML. The extension is small diff --git a/TODO.md b/TODO.md index ab2810c..368785b 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,19 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-04: Debug mode no longer writes RPC API keys to the console + ([#410](https://git.eeqj.de/sneak/AutistMask/issues/410)). `debugFetch` logged + every request's full URL and body, so an RPC endpoint with a key in its path + or query string printed that key on every request. It now logs the HTTP + method, the URL's origin and, for a JSON-RPC call, the method name. The + balance refresh and token lookup log the RPC endpoint by its origin too. A + failed RPC call's error line prints the error's short message, which names the + HTTP status, not its full message, which carries the request URL. A failed + endpoint check in settings names the endpoint by its origin, not the `fetch` + error's message, which carries the whole URL, password included, for a URL + with a user name and password. The README's DEBUG Mode Policy says what debug + mode logs. + - 2026-10-04: A site has at most one connection prompt and one signature prompt open at a time ([#405](https://git.eeqj.de/sneak/AutistMask/issues/405)). Each `eth_requestAccounts` or `personal_sign` call opened another approval window, diff --git a/src/popup/views/confirmTx.js b/src/popup/views/confirmTx.js index 5603865..0c8cacb 100644 --- a/src/popup/views/confirmTx.js +++ b/src/popup/views/confirmTx.js @@ -395,7 +395,7 @@ async function estimateGas(txInfo) { feeWei = gasCostWei; renderValidation(txInfo); } catch (e) { - log.errorf("gas estimation failed:", e.message); + log.errorf("gas estimation failed:", e.shortMessage || e.message); if (pendingTx !== txInfo) return; $("confirm-fee-amount").textContent = "Unable to estimate"; setVisible("confirm-fee-reserve", false); diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index 7bb97ac..6262085 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -16,7 +16,12 @@ const { } = require("../dustThreshold"); const { state, saveState, currentNetwork } = require("../../shared/state"); const { onChainSwitch } = require("../../shared/chainSwitch"); -const { log, debugFetch, setRuntimeDebug } = require("../../shared/log"); +const { + log, + debugFetch, + urlOrigin, + setRuntimeDebug, +} = require("../../shared/log"); const deleteWallet = require("./deleteWallet"); const showPhrase = require("./showPhrase"); const { walletHasRecoveryPhrase } = require("../../shared/wallet"); @@ -272,8 +277,11 @@ function init(ctx) { showFlash("Wrong network: expected " + net.name + "."); return; } - } catch (e) { - log.errorf("RPC validation fetch failed:", e.message); + } catch { + // Not the error's message: fetch puts the whole URL, password and + // key included, in the message of the error it throws for a URL + // with a user name and password or one it cannot parse. + log.errorf("RPC validation fetch failed:", urlOrigin(url)); showFlash("Could not reach endpoint."); return; } @@ -295,8 +303,9 @@ function init(ctx) { showFlash("Endpoint returned HTTP " + resp.status + "."); return; } - } catch (e) { - log.errorf("Blockscout validation failed:", e.message); + } catch { + // Not the error's message, as for the RPC check above. + log.errorf("Blockscout validation failed:", urlOrigin(url)); showFlash("Could not reach endpoint."); return; } diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index 2c2a101..56537f4 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -133,7 +133,7 @@ function startWait(txInfo, txHash, broadcastTime, pollNow) { // failed — which matters most on a resumed wait, where the // first poll is already past the deadline. answered = false; - log.errorf("poll receipt failed:", e.message); + log.errorf("poll receipt failed:", e.shortMessage || e.message); } // The lookup is async: the wait may have ended while it was in // flight, in which case this result must not touch the view. diff --git a/src/shared/addressWarnings.js b/src/shared/addressWarnings.js index 5300b0e..d1a5bc5 100644 --- a/src/shared/addressWarnings.js +++ b/src/shared/addressWarnings.js @@ -75,7 +75,7 @@ async function getFullWarnings(address, provider, options = {}) { }); } } catch (e) { - log.errorf("contract check failed:", e.message); + log.errorf("contract check failed:", e.shortMessage || e.message); } // Skip tx count check for contracts — they may legitimately have @@ -92,7 +92,7 @@ async function getFullWarnings(address, provider, options = {}) { }); } } catch (e) { - log.errorf("tx count check failed:", e.message); + log.errorf("tx count check failed:", e.shortMessage || e.message); } } diff --git a/src/shared/balances.js b/src/shared/balances.js index fcff065..dcbbabf 100644 --- a/src/shared/balances.js +++ b/src/shared/balances.js @@ -10,7 +10,7 @@ const { } = require("ethers"); const { ERC20_ABI } = require("./constants"); const { NETWORKS } = require("./networks"); -const { log, debugFetch } = require("./log"); +const { log, debugFetch, urlOrigin } = require("./log"); const { deriveAddressFromXpub } = require("./wallet"); const { TOKEN_BY_ADDRESS } = require("./tokenList"); const { LOW_HOLDER_THRESHOLD, parseHoldersCount } = require("./holders"); @@ -203,7 +203,7 @@ async function refreshBalances( trackedTokens, networkId, ) { - log.debugf("refreshBalances start, rpc:", rpcUrl); + log.debugf("refreshBalances start, rpc:", urlOrigin(rpcUrl)); const provider = getProvider(rpcUrl, networkId); const updates = []; @@ -246,7 +246,7 @@ async function refreshBalances( log.errorf( "ENS reverse failed", addr.address, - e.message, + e.shortMessage || e.message, ); // Keep existing addr.ensName if we had one }), @@ -280,7 +280,7 @@ async function refreshBalances( // Look up token metadata from its contract. // Calls symbol() and decimals() to verify it implements ERC-20. async function lookupTokenInfo(contractAddress, rpcUrl, networkId) { - log.debugf("lookupTokenInfo", contractAddress, "rpc:", rpcUrl); + log.debugf("lookupTokenInfo", contractAddress, "rpc:", urlOrigin(rpcUrl)); const provider = getProvider(rpcUrl, networkId); const contract = new Contract(contractAddress, ERC20_ABI, provider); @@ -305,7 +305,10 @@ async function lookupTokenInfo(contractAddress, rpcUrl, networkId) { name = await contract.name(); log.debugf("name() =", name); } catch (e) { - log.warnf("name() failed, using symbol as name:", e.message); + log.warnf( + "name() failed, using symbol as name:", + e.shortMessage || e.message, + ); name = symbol; } diff --git a/src/shared/ens.js b/src/shared/ens.js index 4b95660..62e400f 100644 --- a/src/shared/ens.js +++ b/src/shared/ens.js @@ -42,7 +42,11 @@ async function resolveEnsName(address, rpcUrl, networkId) { setCache(address, name); return name; } catch (e) { - log.errorf("ENS reverse lookup failed", address, e.message); + log.errorf( + "ENS reverse lookup failed", + address, + e.shortMessage || e.message, + ); // Don't cache failures — let subsequent lookups retry return null; } diff --git a/src/shared/log.js b/src/shared/log.js index 551fb20..e3d8168 100644 --- a/src/shared/log.js +++ b/src/shared/log.js @@ -42,14 +42,34 @@ const log = { }, }; -// Fetch wrapper that debug-logs every request and response. +// The origin (scheme, host and port) of a URL, for logging in place of the +// URL: RPC providers put API keys in the path or the query string, and a URL +// can carry a user name and password, which the origin leaves out. A URL that +// does not parse gives "", so logging never stops a request. +function urlOrigin(url) { + try { + return new URL(url).origin; + } catch { + return ""; + } +} + +// Fetch wrapper that debug-logs every request and response. It logs the +// URL's origin and, for a JSON-RPC body, the method name: never the full URL +// or body, which can carry an API key or a signed transaction. async function debugFetch(url, opts) { const method = (opts && opts.method) || "GET"; - const body = opts && opts.body; - log.debugf("fetch →", method, url, body || ""); + const origin = urlOrigin(url); + let rpcMethod = ""; + try { + rpcMethod = JSON.parse(opts.body).method || ""; + } catch { + // no body, or a body that is not JSON + } + log.debugf("fetch →", method, origin, rpcMethod); const resp = await fetch(url, opts); - log.debugf("fetch ←", resp.status, url); + log.debugf("fetch ←", resp.status, origin); return resp; } -module.exports = { log, debugFetch, setRuntimeDebug, isDebug }; +module.exports = { log, debugFetch, urlOrigin, setRuntimeDebug, isDebug }; diff --git a/tests/debugFetch.test.js b/tests/debugFetch.test.js new file mode 100644 index 0000000..ec34bec --- /dev/null +++ b/tests/debugFetch.test.js @@ -0,0 +1,53 @@ +// What debugFetch writes to the console in debug mode. +// +// RPC providers put the API key in the URL's path or query string, and the +// debug log used to print the whole URL and request body, so turning debug +// mode on wrote the key to the console +// (https://git.eeqj.de/sneak/AutistMask/issues/410). The log now names the +// HTTP method, the URL's origin and the JSON-RPC method, and nothing else of +// the request. + +const { debugFetch, urlOrigin, setRuntimeDebug } = require("../src/shared/log"); + +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; + setRuntimeDebug(false); + jest.restoreAllMocks(); +}); + +test("logs the origin and JSON-RPC method, not the key in the URL", async () => { + setRuntimeDebug(true); + const consoleLog = jest.spyOn(console, "log").mockImplementation(() => {}); + globalThis.fetch = jest.fn(async () => ({ status: 200 })); + + await debugFetch( + "https://rpc.example.invalid/v3/PATHKEY123?token=QUERYTOKEN456", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "eth_chainId", + params: [], + }), + }, + ); + + const logged = consoleLog.mock.calls.flat().join(" "); + expect(logged).not.toContain("PATHKEY123"); + expect(logged).not.toContain("QUERYTOKEN456"); + expect(logged).toContain("https://rpc.example.invalid"); + expect(logged).toContain("eth_chainId"); +}); + +test("the origin leaves out a user name and password in the URL", () => { + expect( + urlOrigin("https://user:SECRETPASS@rpc.example.invalid/v3/KEY"), + ).toBe("https://rpc.example.invalid"); + expect(urlOrigin("wss://user:SECRETPASS@rpc.example.invalid:8546/")).toBe( + "wss://rpc.example.invalid:8546", + ); +}); diff --git a/tests/rpcErrorLog.test.js b/tests/rpcErrorLog.test.js new file mode 100644 index 0000000..f06284a --- /dev/null +++ b/tests/rpcErrorLog.test.js @@ -0,0 +1,105 @@ +// What reaches the console when the RPC endpoint answers with an HTTP error. +// +// RPC providers put the API key in the endpoint URL's path or query string. +// When the endpoint answers with an HTTP error (a wrong or expired key, a rate +// limit, a server error), the error ethers throws carries the full request URL +// in its message, so a line logging that message printed the key +// (https://git.eeqj.de/sneak/AutistMask/issues/410). Those lines log the +// error's short message, which names the HTTP status and not the URL. +// +// The real ethers provider runs; only its HTTP transport is replaced, by one +// that answers every request with 401 Unauthorized. Debug mode is on, so +// every log level is printed. + +const { FetchRequest } = require("ethers"); +const { + getProvider, + lookupTokenInfo, + refreshBalances, +} = require("../src/shared/balances"); +const { getFullWarnings } = require("../src/shared/addressWarnings"); +const { resolveEnsName } = require("../src/shared/ens"); +const { setRuntimeDebug } = require("../src/shared/log"); + +const RPC_URL = "https://rpc.example.invalid/v3/PATHKEY123?token=QUERYTOKEN456"; +const ADDRESS = "0x1111111111111111111111111111111111111111"; + +const realFetch = globalThis.fetch; +let printed; + +beforeEach(() => { + setRuntimeDebug(true); + printed = []; + for (const method of ["log", "warn", "error"]) { + jest.spyOn(console, method).mockImplementation((...args) => { + printed.push(args.map(String).join(" ")); + }); + } + FetchRequest.registerGetUrl(async () => ({ + statusCode: 401, + statusMessage: "Unauthorized", + headers: {}, + body: new Uint8Array(), + })); + // The explorer requests the balance refresh makes go nowhere. + globalThis.fetch = jest.fn(async () => { + throw new Error("tests must not perform network requests"); + }); +}); + +afterEach(() => { + FetchRequest.registerGetUrl(FetchRequest.createGetUrlFunc()); + globalThis.fetch = realFetch; + setRuntimeDebug(false); + jest.restoreAllMocks(); +}); + +// The line carrying `label` was printed and names the HTTP status, and nothing +// printed carries the key. +function expectFailureLoggedWithoutKey(label) { + const line = printed.find((text) => text.includes(label)); + expect(line).toContain("401"); + const all = printed.join("\n"); + expect(all).not.toContain("PATHKEY123"); + expect(all).not.toContain("QUERYTOKEN456"); +} + +test("ethers puts the URL in the error message, not in the short message", async () => { + const provider = getProvider(RPC_URL, "mainnet"); + const error = await provider.getCode(ADDRESS).catch((e) => e); + expect(error.message).toContain("PATHKEY123"); + expect(error.shortMessage).not.toContain("PATHKEY123"); +}); + +test("the recipient checks before a send", async () => { + await getFullWarnings(ADDRESS, getProvider(RPC_URL, "mainnet")); + expectFailureLoggedWithoutKey("contract check failed"); + expectFailureLoggedWithoutKey("tx count check failed"); +}); + +test("the ENS reverse lookup", async () => { + expect(await resolveEnsName(ADDRESS, RPC_URL, "mainnet")).toBeNull(); + expectFailureLoggedWithoutKey("ENS reverse lookup failed"); +}); + +test("the balance refresh", async () => { + const wallets = [{ addresses: [{ address: ADDRESS }] }]; + await refreshBalances( + wallets, + RPC_URL, + "https://explorer.example.invalid/api/v2", + [], + "mainnet", + ); + expectFailureLoggedWithoutKey("ETH balance failed"); + expectFailureLoggedWithoutKey("ENS reverse failed"); +}); + +// The lookup's first line, at debug level, names the RPC endpoint; the check +// of everything printed covers it too. +test("the token lookup", async () => { + await expect(lookupTokenInfo(ADDRESS, RPC_URL, "mainnet")).rejects.toThrow( + "Not a valid ERC-20 token", + ); + expectFailureLoggedWithoutKey("symbol() failed:"); +}); diff --git a/tests/sendDisplayFloor.test.js b/tests/sendDisplayFloor.test.js index c366314..1e0b885 100644 --- a/tests/sendDisplayFloor.test.js +++ b/tests/sendDisplayFloor.test.js @@ -66,6 +66,7 @@ jest.mock("../src/shared/log", () => ({ status: 200, json: async () => mockExplorer.items, })), + urlOrigin: () => "", setRuntimeDebug: () => {}, isDebug: () => false, })); diff --git a/tests/settingsEndpointCheck.test.js b/tests/settingsEndpointCheck.test.js new file mode 100644 index 0000000..988097f --- /dev/null +++ b/tests/settingsEndpointCheck.test.js @@ -0,0 +1,148 @@ +// What reaches the console when an endpoint check in Settings fails. +// +// fetch refuses a URL with a user name and password in it, or one it cannot +// parse, with an error whose message carries the whole URL: the password, and +// any API key in the path or query string. The checks behind the RPC and +// Blockscout Save buttons printed that message +// (https://git.eeqj.de/sneak/AutistMask/issues/410); they now name the +// endpoint by its origin. +// +// The real fetch runs; it throws before making any request. Debug mode is on, +// so every log level is printed. + +const SECRETS = ["SECRETPASS789", "PATHKEY123", "QUERYTOKEN456"]; +const RPC_WITH_PASSWORD = + "https://user:SECRETPASS789@rpc.example.invalid/v3/PATHKEY123?token=QUERYTOKEN456"; +// Port 99999 is out of range, so the URL does not parse. +const RPC_UNPARSEABLE = + "https://rpc.example.invalid:99999/v3/PATHKEY123?token=QUERYTOKEN456"; +const BLOCKSCOUT_WITH_PASSWORD = + "https://user:SECRETPASS789@explorer.example.invalid/PATHKEY123/api/v2"; + +const SAVED_RPC = "https://saved-rpc.example.invalid"; +const SAVED_BLOCKSCOUT = "https://saved-explorer.example.invalid/api/v2"; + +let elements; +let flashes; +let printed; +let state; + +// A stand-in for one DOM node: enough of an element for init() to set +// properties on it and hang listeners off it. +function fakeElement() { + return { + value: "", + checked: false, + textContent: "", + href: "", + style: {}, + dataset: {}, + classList: { add() {}, remove() {} }, + listeners: {}, + addEventListener(event, handler) { + this.listeners[event] = handler; + }, + querySelectorAll: () => [], + }; +} + +function element(id) { + return (elements[id] ||= fakeElement()); +} + +function loadSettingsView() { + elements = {}; + flashes = []; + + jest.resetModules(); + + jest.doMock("../src/popup/views/helpers", () => ({ + $: element, + showView: () => {}, + updateDebugBanner: () => {}, + showFlash: (msg) => flashes.push(msg), + escapeHtml: (s) => s, + flashCopyFeedback: () => {}, + goBack: () => {}, + pushCurrentView: () => {}, + onViewLeave: () => {}, + VIEWS: [], + })); + + state = require("../src/shared/state").state; + state.rpcUrl = SAVED_RPC; + state.blockscoutUrl = SAVED_BLOCKSCOUT; + require("../src/shared/log").setRuntimeDebug(true); + + require("../src/popup/views/settings").init({}); +} + +async function save(fieldId, buttonId, typed) { + element(fieldId).value = typed; + await element(buttonId).listeners.click(); +} + +// The check failed, nothing was saved, and nothing printed carries the +// password or the key. +function expectFailedWithoutSecrets(label) { + expect(flashes).toContain("Could not reach endpoint."); + expect(state.rpcUrl).toBe(SAVED_RPC); + expect(state.blockscoutUrl).toBe(SAVED_BLOCKSCOUT); + const line = printed.find((text) => text.includes(label)); + expect(line).toBeDefined(); + const all = printed.join("\n"); + for (const secret of SECRETS) { + expect(all).not.toContain(secret); + } + return line; +} + +beforeEach(() => { + printed = []; + for (const method of ["log", "warn", "error"]) { + jest.spyOn(console, method).mockImplementation((...args) => { + printed.push(args.map(String).join(" ")); + }); + } + globalThis.chrome = { + runtime: { sendMessage: () => {} }, + storage: { local: { get: async () => ({}), set: async () => {} } }, + }; +}); + +afterEach(() => { + jest.dontMock("../src/popup/views/helpers"); + delete globalThis.chrome; + jest.restoreAllMocks(); +}); + +test("fetch puts the whole URL in the error it throws for such a URL", async () => { + for (const url of [RPC_WITH_PASSWORD, RPC_UNPARSEABLE]) { + const error = await fetch(url).catch((e) => e); + expect(error.message).toContain("PATHKEY123"); + } +}); + +test("the RPC check of a URL with a user name and password", async () => { + loadSettingsView(); + await save("settings-rpc", "btn-save-rpc", RPC_WITH_PASSWORD); + const line = expectFailedWithoutSecrets("RPC validation fetch failed"); + expect(line).toContain("https://rpc.example.invalid"); +}); + +test("the RPC check of a URL that does not parse", async () => { + loadSettingsView(); + await save("settings-rpc", "btn-save-rpc", RPC_UNPARSEABLE); + expectFailedWithoutSecrets("RPC validation fetch failed"); +}); + +test("the Blockscout check of a URL with a user name and password", async () => { + loadSettingsView(); + await save( + "settings-blockscout", + "btn-save-blockscout", + BLOCKSCOUT_WITH_PASSWORD, + ); + const line = expectFailedWithoutSecrets("Blockscout validation failed"); + expect(line).toContain("https://explorer.example.invalid"); +}); diff --git a/tests/symbolSpoof.test.js b/tests/symbolSpoof.test.js index 94163be..b0b6090 100644 --- a/tests/symbolSpoof.test.js +++ b/tests/symbolSpoof.test.js @@ -44,6 +44,7 @@ jest.mock("../src/shared/log", () => ({ errorf: () => {}, }, debugFetch: jest.fn(), + urlOrigin: () => "", setRuntimeDebug: () => {}, isDebug: () => false, }));