From 49a7da87e81ef66a0015d094a8f49a0c823a2157 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 06:41:39 +0200 Subject: [PATCH] harden: list and end site connections made without Remember in Settings (closes #406) A site allowed without "Remember" lives only in the background's in-memory connectedSites map. Settings never listed it, and AUTISTMASK_REMOVE_SITE, sent on every remove, did nothing, so the user could not end such a connection. Settings now asks the background for those sites and lists them under Connected Sites. Removing a site from Allowed Sites or Connected Sites drops its remembered entry under every address and sends AUTISTMASK_REMOVE_SITE with the hostname; the background deletes every matching connectedSites entry and sends accountsChanged with an empty list to the site's tabs. Only the extension's own pages may send either message. Model: opus-5-5 --- README.md | 24 +++- TODO.md | 13 +++ src/background/index.js | 42 ++++++- src/popup/index.html | 9 ++ src/popup/views/settings.js | 74 +++++++++---- tests/backgroundApproval.test.js | 185 +++++++++++++++++++++++++++++-- 6 files changed, 308 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index c7cf35c..bb0d196 100644 --- a/README.md +++ b/README.md @@ -1582,8 +1582,14 @@ view would leave a wallet one click from deletion. a value carrying its unit, hex (`0x10`) or exponent (`1e3`) notation — is refused with a flash message and the field snaps back to the stored threshold, so a number the user did not type is never stored. - - Allowed Sites: list with remove buttons - - Denied Sites: list with remove buttons + - Allowed Sites: the hostnames remembered as allowed, under any address, + with remove buttons + - Connected Sites: the hostnames of the sites allowed without "Remember my + choice" that are still connected, with remove buttons. Only the background + holds these, in memory, and Settings asks it for them with + `AUTISTMASK_GET_CONNECTED_SITES` + - Denied Sites: the hostnames remembered as denied, under any address, with + remove buttons - About: project link, license, author, version, release date, and the commit, which links to the commit in the repository - Debug: hidden until revealed, then an "Enable debug mode" checkbox that @@ -1594,8 +1600,15 @@ view would leave a wallet one click from deletion. - `[recovery phrase]` on an HD wallet → **ShowRecoveryPhrase** - `[x]` on a wallet → **DeleteWallet** - Tap wallet name → inline rename field (no screen change) - - `[x]` on a tracked token or a site → removes it in place (no screen - change) + - `[x]` on a tracked token → removes it in place (no screen change) + - `[x]` on an allowed or connected site → disconnects that site, in place: + its hostname is dropped from Allowed Sites under every address, and + `AUTISTMASK_REMOVE_SITE` has the background end every connection approved + without "Remember" from an origin with that hostname, under any address, + and send `accountsChanged` with an empty list to the site's open tabs. + Only the extension's own pages may send either message + - `[x]` on a denied site → forgets the refusal, in place; it connects + nothing and tells the background nothing - Ten clicks on the version → reveals the Debug well (no screen change) - "Back" (or Settings gear again) → previous screen (Home) @@ -1790,7 +1803,8 @@ view would leave a wallet one click from deletion. - **Transitions**: - "Allow" / "Deny" → closes popup (returns result to background script; the choice is persisted to the allowed or denied list when "Remember" is - checked) + checked, and an "Allow" without it is listed under Connected Sites in + **Settings**) - Popup closed without answering → treated as a denial #### TxApproval (`approve-tx`) diff --git a/TODO.md b/TODO.md index f06c9c2..49525b6 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: Settings lists the sites connected without "Remember", and + removing a site there disconnects it + ([#406](https://git.eeqj.de/sneak/AutistMask/issues/406)). Such a connection + lives only in the background's in-memory `connectedSites` map, so Settings + never showed it and the user could not end it; `AUTISTMASK_REMOVE_SITE`, sent + on every remove, did nothing. Settings now asks the background for those sites + (`AUTISTMASK_GET_CONNECTED_SITES`) and lists them under Connected Sites. + Removing a site from Allowed Sites or Connected Sites drops its remembered + entry under every address and sends `AUTISTMASK_REMOVE_SITE` with the + hostname; the background deletes every `connectedSites` entry for that + hostname and sends `accountsChanged` with an empty list to its open tabs. Only + the extension's own pages may send either message. Removing a denied site no + longer sends it, since forgetting a refusal ends no connection. - 2026-10-04: Removing an address or deleting a wallet ends every site connection approved without "Remember" for the addresses removed ([#245](https://git.eeqj.de/sneak/AutistMask/issues/245)). Such a connection diff --git a/src/background/index.js b/src/background/index.js index 8087341..42b5c27 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -1110,6 +1110,24 @@ async function broadcastAccountsChanged() { } } +// Tell every open tab of a site Settings removed that it has no account. +async function broadcastSiteRemoved(hostname) { + let tabs; + try { + tabs = await tabsQuery({}); + } catch { + return; + } + for (const tab of tabs) { + if (!tab.url || extractHostname(tab.url) !== hostname) continue; + tabsSendMessage(tab.id, { + type: "AUTISTMASK_EVENT", + eventName: "accountsChanged", + data: [], + }).catch(() => {}); + } +} + // Background balance refresh: every 60 seconds when the popup isn't open. // When the popup IS open, its 10-second interval keeps lastBalanceRefresh // fresh, so this naturally skips. @@ -1306,6 +1324,8 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { "AUTISTMASK_TX_RESPONSE", "AUTISTMASK_SIGN_RESPONSE", "AUTISTMASK_ADDRESSES_REMOVED", + "AUTISTMASK_GET_CONNECTED_SITES", + "AUTISTMASK_REMOVE_SITE", ]; if (POPUP_ONLY_TYPES.includes(msg.type) && !isExtensionSender(sender)) { sendResponse({ error: "Unauthorized sender" }); @@ -1662,8 +1682,28 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { return false; } + // Settings lists the sites connected without "Remember", which only this + // worker knows. The origin is what precedes the key's last colon. + if (msg.type === "AUTISTMASK_GET_CONNECTED_SITES") { + sendResponse( + Object.keys(connectedSites).map((key) => + extractHostname(key.slice(0, key.lastIndexOf(":"))), + ), + ); + return false; + } + + // Settings removed this site and has already dropped its remembered + // entries. Its connections approved without "Remember" end here, under + // every address, and its open tabs are told it has no account. if (msg.type === "AUTISTMASK_REMOVE_SITE") { - // Popup already saved state; nothing else needed + for (const key of Object.keys(connectedSites)) { + const origin = key.slice(0, key.lastIndexOf(":")); + if (extractHostname(origin) === msg.hostname) { + delete connectedSites[key]; + } + } + broadcastSiteRemoved(msg.hostname); return false; } }); diff --git a/src/popup/index.html b/src/popup/index.html index 204ff75..e645e3f 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -1064,6 +1064,15 @@
+
+

Connected Sites

+

+ Sites you allowed without "Remember my choice". + Switching address disconnects them. +

+
+
+

Denied Sites

diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index 06fd3d4..4049d55 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -29,46 +29,63 @@ const { GITEA_COMMIT_URL, } = require("../../shared/buildInfo"); -const { notify } = require("../../shared/browserApi"); +const { notify, sendMessage } = require("../../shared/browserApi"); let versionClickCount = 0; let versionClickTimer = null; -function renderSiteList(containerId, siteMap, stateKey) { +// One row per hostname, however many addresses or origins it appears under, +// each with an [x] that hands it to onRemove. +function renderSiteList(containerId, hostnames, onRemove) { const container = $(containerId); - const hostnames = [...new Set(Object.values(siteMap).flat())]; - if (hostnames.length === 0) { + const unique = [...new Set(hostnames)]; + if (unique.length === 0) { container.innerHTML = '

None

'; return; } let html = ""; - hostnames.forEach((hostname) => { + unique.forEach((hostname) => { html += `
`; // A hostname the URL parser produced cannot carry a delimiter, so // this is escaped for the rule rather than for a known hole — the // rule being that nothing reaches innerHTML unescaped. html += `${escapeHtml(hostname)}`; - html += ``; + html += ``; html += `
`; }); container.innerHTML = html; container.querySelectorAll(".btn-remove-site").forEach((btn) => { - btn.addEventListener("click", async () => { - const key = btn.dataset.key; - const host = btn.dataset.hostname; - for (const addr of Object.keys(state[key])) { - state[key][addr] = state[key][addr].filter((h) => h !== host); - if (state[key][addr].length === 0) { - delete state[key][addr]; - } - } - await saveState(); - notify({ type: "AUTISTMASK_REMOVE_SITE" }); - renderSiteList(containerId, state[key], key); - }); + btn.addEventListener("click", () => onRemove(btn.dataset.hostname)); }); } +// Drop a hostname from a remembered site list under every address. +function forgetHostname(siteMap, hostname) { + for (const addr of Object.keys(siteMap)) { + siteMap[addr] = siteMap[addr].filter((h) => h !== hostname); + if (siteMap[addr].length === 0) { + delete siteMap[addr]; + } + } +} + +// Removing a site from Allowed Sites or Connected Sites disconnects it: it is +// no longer allowed under any address, and the background ends its +// connections approved without "Remember" and tells its open tabs. +async function removeAllowedSite(hostname) { + forgetHostname(state.allowedSites, hostname); + await saveState(); + notify({ type: "AUTISTMASK_REMOVE_SITE", hostname }); + await renderSiteLists(); +} + +// Removing a denied site only forgets the refusal; it connects nothing. +async function removeDeniedSite(hostname) { + forgetHostname(state.deniedSites, hostname); + await saveState(); + await renderSiteLists(); +} + function renderTrackedTokens() { const container = $("settings-tracked-tokens"); if (state.trackedTokens.length === 0) { @@ -202,13 +219,24 @@ function show() { showView("settings"); } -function renderSiteLists() { +async function renderSiteLists() { renderSiteList( "settings-allowed-sites", - state.allowedSites, - "allowedSites", + Object.values(state.allowedSites).flat(), + removeAllowedSite, + ); + renderSiteList( + "settings-denied-sites", + Object.values(state.deniedSites).flat(), + removeDeniedSite, + ); + // Sites allowed without "Remember" are held only by the background, in + // memory, so it is asked for them. + renderSiteList( + "settings-connected-sites", + await sendMessage({ type: "AUTISTMASK_GET_CONNECTED_SITES" }), + removeAllowedSite, ); - renderSiteList("settings-denied-sites", state.deniedSites, "deniedSites"); } function init(ctx) { diff --git a/tests/backgroundApproval.test.js b/tests/backgroundApproval.test.js index 18c7375..fe430f0 100644 --- a/tests/backgroundApproval.test.js +++ b/tests/backgroundApproval.test.js @@ -2101,6 +2101,16 @@ describe("a site connection decided as the popup closes", () => { }); }); +// What a site is told when it asks which account it may use. +async function siteAccounts(bg, origin) { + const { sendResponse } = bg.send( + { type: "AUTISTMASK_RPC", method: "eth_accounts", params: [] }, + { origin: origin || FRESH_ORIGIN }, + ); + await settle(); + return sendResponse.mock.calls[0][0]; +} + // A site connected without "Remember" is held only in the background's memory, // keyed to the address it was connected to. Removing that address, or the // wallet holding it, must end the connection as part of the removal itself. @@ -2136,16 +2146,6 @@ describe("removing an address ends a site's connection to it", () => { return bg; } - // What FRESH_ORIGIN is told when it asks which account it may use. - async function siteAccounts(bg) { - const { sendResponse } = bg.send( - { type: "AUTISTMASK_RPC", method: "eth_accounts", params: [] }, - { origin: FRESH_ORIGIN }, - ); - await settle(); - return sendResponse.mock.calls[0][0]; - } - test("removing the connected address ends the connection", async () => { const bg = await connectedBackground(); expect(await siteAccounts(bg)).toEqual({ result: [signer.address] }); @@ -2192,3 +2192,168 @@ describe("removing an address ends a site's connection to it", () => { expect(await siteAccounts(bg)).toEqual({ result: [signer.address] }); }); }); + +// Settings lists the sites allowed without "Remember", which only the +// background holds, and removing a site there, from either list, disconnects +// it. These drive the real Settings view against the real background and +// click the [x] the user clicks. +describe("removing a site in Settings disconnects it", () => { + // FRESH_ORIGIN on another port, so its hostname is FRESH_ORIGIN's. + const FRESH_OTHER_PORT = "https://fresh.example:8443"; + + // A site list's container. Its [x] buttons, data attributes and all, are + // read back out of the rows the view wrote into it, so clicking one runs + // the handler the view attached to it. + function fakeSiteList() { + const list = { + innerHTML: "", + buttons: [], + querySelectorAll() { + const tags = list.innerHTML.match(/]*>/g) || []; + list.buttons = tags.map((tag) => ({ + dataset: Object.fromEntries( + [...tag.matchAll(/data-(\w+)="([^"]*)"/g)].map( + (match) => [match[1], match[2]], + ), + ), + addEventListener(event, handler) { + this[event] = handler; + }, + })); + return list.buttons; + }, + }; + return list; + } + + // The hostnames a site list shows. + function listed(list) { + return [...list.innerHTML.matchAll(/data-hostname="([^"]*)"/g)].map( + (match) => match[1], + ); + } + + // A site connected the way the user does it, in the approval popup. + async function connect(bg, origin, remember) { + const pending = bg.requestSite(origin); + await settle(); + bg.connectApproval(pending.id()).decide(true, remember); + await settle(); + expect(pending.result()).toEqual({ result: [signer.address] }); + } + + // Settings, opened over the background's storage and wired to it the way + // the popup is: what Settings sends reaches the background from the + // extension's own page, and the answer comes back. + async function openSettings(bg) { + const lists = {}; + const element = (id) => (lists[id] ||= fakeSiteList()); + global.document = { getElementById: element }; + global.chrome.runtime.sendMessage = (msg, callback) => { + const { sendResponse } = bg.send(msg, bg.fromPopup); + if (callback) callback(sendResponse.mock.calls[0]?.[0]); + }; + await require("../src/shared/state").loadState(); + await require("../src/popup/views/settings").renderSiteLists(); + return { + allowed: element("settings-allowed-sites"), + connected: element("settings-connected-sites"), + // Click the [x] beside a site, and let what it sends run. + remove: async (list, hostname) => { + expect(listed(list)).toContain(hostname); + const button = list.buttons.find( + (b) => b.dataset.hostname === hostname, + ); + await button.click(); + await settle(); + }, + }; + } + + afterEach(() => { + delete global.document; + }); + + test("Settings lists a site connected without Remember", async () => { + const bg = loadBackground({ actionPopup: true }); + await connect(bg, FRESH_ORIGIN, false); + + const settings = await openSettings(bg); + + expect(listed(settings.connected)).toEqual(["fresh.example"]); + expect(listed(settings.allowed)).toEqual([HOSTNAME]); + }); + + test("removing a site connected without Remember disconnects it and tells its tabs", async () => { + const bg = loadBackground({ actionPopup: true }); + await connect(bg, FRESH_ORIGIN, false); + const sentToTabs = []; + global.chrome.tabs = { + query: (q, cb) => + cb([ + { id: 1, url: FRESH_ORIGIN + "/app" }, + { id: 2, url: ORIGIN + "/app" }, + ]), + sendMessage: (tabId, msg, cb) => { + sentToTabs.push({ tabId, msg }); + cb(); + }, + }; + const settings = await openSettings(bg); + + await settings.remove(settings.connected, "fresh.example"); + + expect(await siteAccounts(bg)).toEqual({ result: [] }); + expect(sentToTabs).toEqual([ + { + tabId: 1, + msg: { + type: "AUTISTMASK_EVENT", + eventName: "accountsChanged", + data: [], + }, + }, + ]); + expect(listed(settings.connected)).toEqual([]); + // The other site is untouched. + expect(await siteAccounts(bg, ORIGIN)).toEqual({ + result: [signer.address], + }); + }); + + // The same hostname can hold both kinds of connection: one origin allowed + // without Remember, then another, on a different port, allowed with it. + test("removing a remembered site also ends its connection made without Remember", async () => { + const bg = loadBackground({ actionPopup: true }); + await connect(bg, FRESH_ORIGIN, false); + await connect(bg, FRESH_OTHER_PORT, true); + const settings = await openSettings(bg); + + await settings.remove(settings.allowed, "fresh.example"); + + expect(await siteAccounts(bg, FRESH_OTHER_PORT)).toEqual({ + result: [], + }); + expect(await siteAccounts(bg, FRESH_ORIGIN)).toEqual({ result: [] }); + }); + + test("a page can neither remove a site nor list the connected ones", async () => { + const bg = loadBackground({ actionPopup: true }); + await connect(bg, FRESH_ORIGIN, false); + const page = { url: FRESH_ORIGIN + "/index.html" }; + + const remove = bg.send( + { type: "AUTISTMASK_REMOVE_SITE", hostname: "fresh.example" }, + page, + ); + const list = bg.send({ type: "AUTISTMASK_GET_CONNECTED_SITES" }, page); + + expect(remove.sendResponse).toHaveBeenCalledWith({ + error: "Unauthorized sender", + }); + expect(list.sendResponse).toHaveBeenCalledWith({ + error: "Unauthorized sender", + }); + expect(await siteAccounts(bg)).toEqual({ result: [signer.address] }); + }); +});