diff --git a/README.md b/README.md index 9c4364e..c739ddc 100644 --- a/README.md +++ b/README.md @@ -2168,6 +2168,16 @@ 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 and token lookup name the RPC endpoint by its origin too. The URL's path +and query string, where RPC providers put API keys, 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..89a4edd 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,16 @@ 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. 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/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..15c2a1c 100644 --- a/src/shared/log.js +++ b/src/shared/log.js @@ -42,14 +42,33 @@ 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. 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..20f5165 --- /dev/null +++ b/tests/debugFetch.test.js @@ -0,0 +1,44 @@ +// 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, 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"); +}); diff --git a/tests/rpcErrorLog.test.js b/tests/rpcErrorLog.test.js new file mode 100644 index 0000000..9fcbd2f --- /dev/null +++ b/tests/rpcErrorLog.test.js @@ -0,0 +1,92 @@ +// 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, 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"); +}); 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/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, }));