Compare commits

...
2 Commits
Author SHA1 Message Date
sneak da464201f3 chore: keep the internal view id out of the release banner (closes #375)
check / check (push) Failing after 1s
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 0s
The debug/testnet banner appended the active view's internal id, so the
user saw text like "[TESTNET] (approve-tx)" — developer vocabulary, and on
the approval screen it sat directly above the carefully worded line stating
what is being authorized. The view id is now gated on the compile-time
DEBUG constant instead of isDebug(), so it survives only in a debug build.
A testnet or the runtime debug toggle still raises the banner, but without
the view id, which is what a release build shows.

Model: opus-4-8
2026-09-21 13:02:24 +00:00
clawbot 99292b9188 fix: honour a transaction response only for a transaction approval (closes #262)
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
check / check (push) Successful in 1m42s
The liveness fix this issue describes — settle 4001 on release when the window
a retry would use is gone — already landed with
#271. This completes the rest.

AUTISTMASK_TX_RESPONSE now refuses any approval that is not a transaction
approval, so a reject can no longer retire a sign or connection approval, and a
signed artifact never runs the broadcast path against one — which before only
failed closed by throwing deeper in. Tests pin the site-connection port's
approve, reject and disconnect paths against a transaction approval broadcasting
behind them: each is declined and the dApp still receives its broadcast result.

Model: opus-4-8
2026-09-21 09:54:40 +02:00
5 changed files with 241 additions and 1 deletions
+24
View File
@@ -45,6 +45,30 @@ but the review is broader than any of them.
# Completed Steps
- 2026-09-21: The debug/testnet banner no longer shows the internal view id to
the user in a release build
([#375](https://git.eeqj.de/sneak/AutistMask/issues/375)). The banner appended
the active view's id (e.g. `[TESTNET] (approve-tx)`), which is developer
vocabulary sitting directly above the approval screen's carefully worded
authorization text. The suffix is now gated on the compile-time `DEBUG`
constant rather than `isDebug()`, so it survives only in a debug build; a
testnet or the runtime debug toggle still raises the banner but without the
view id.
- 2026-09-21: A transaction response is honoured only for a transaction
approval, and the three remaining approval-settlement paths are pinned
([#262](https://git.eeqj.de/sneak/AutistMask/issues/262)). The liveness fix
the issue asks for — settle `4001` on release when the window it would be
retried in is gone — already landed with
[#271](https://git.eeqj.de/sneak/AutistMask/issues/271); this closes the rest.
`AUTISTMASK_TX_RESPONSE` now refuses any approval that is not a transaction
approval, so a reject no longer retires a sign or connection approval and a
signed artifact never runs the broadcast path against one, which before only
failed closed by throwing deeper in. Tests pin the site-connection port's
approve, reject and disconnect paths against a transaction approval
broadcasting behind them: each is declined and the dApp still receives its
broadcast result.
- 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the
common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The
wallet list was the reported case: the address shared one row with the
+9
View File
@@ -1344,6 +1344,15 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
const approval = pendingApprovals[msg.id];
if (!approval) return false;
// This message signs and broadcasts a transaction, so it is honoured
// only for a transaction approval. A sign or connection approval
// carries no approvedTx, and reaching the broadcast path with one used
// to fail closed by throwing deeper in; refusing here keeps a future
// refactor from turning that incidental throw into a live path, and
// keeps a reject on this message from retiring an approval of another
// kind.
if (approval.type !== "tx") return false;
// A reject arriving while an attempt holds the approval is refused,
// not honoured: the attempt is on its way to broadcasting the
// transaction, and resolving 4001 here would tell the page the request
+6 -1
View File
@@ -12,6 +12,7 @@
// escapeHtml lives in src/shared/html.js, where the escape and the
// reasoning behind it are; it is re-exported below so views keep importing
// it from here.
const { DEBUG } = require("../../shared/constants");
const { escapeHtml } = require("../../shared/html");
const { isDebug } = require("../../shared/log");
const { formatUsd, getPrice } = require("../../shared/prices");
@@ -119,7 +120,11 @@ function updateDebugBanner(viewName) {
"background:#c00;color:#fff;text-align:center;font-size:10px;padding:1px 0;font-family:monospace;position:sticky;top:0;z-index:9999;";
document.body.prepend(banner);
}
const suffix = viewName ? " (" + viewName + ")" : "";
// The view id is internal vocabulary; it helps while developing but
// means nothing to a user. Only a debug build appends it, gated on the
// compile-time DEBUG constant so a release build never shows it — not
// isDebug(), which is also true for a testnet or the runtime toggle.
const suffix = DEBUG && viewName ? " (" + viewName + ")" : "";
if (debug && net.isTestnet) {
banner.textContent = "DEBUG / INSECURE [TESTNET]" + suffix;
} else if (net.isTestnet) {
+143
View File
@@ -1541,6 +1541,149 @@ describe("a claimed approval outlives every other retirement path", () => {
});
});
// The approval popup connects a port named for its approval whatever the
// approval's kind, so a decision or a disconnect on that port can reach a
// transaction approval. Both must be declined: the port decides only
// site-connection approvals, and settling a transaction approval it does not
// own — while an attempt is broadcasting behind it — is the round-3 fund-loss
// bug, where the page is told the request was rejected as the transaction goes
// out. These three paths route through settleApproval() and, before this
// suite, were exercised only against site approvals.
describe("the site-connection port never retires a transaction approval", () => {
async function txMidBroadcast() {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
const first = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
return { bg, pending, id, inFlight, first };
}
test("an approve on the port does not settle it", async () => {
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
bg.connectApproval(id).decide(true, false);
await settle();
expect(pending.result()).toBeNull();
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
});
test("a reject on the port does not settle it", async () => {
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
bg.connectApproval(id).decide(false, false);
await settle();
expect(pending.result()).toBeNull();
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
});
test("a port disconnect does not settle it", async () => {
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
bg.connectApproval(id).disconnect();
await settle();
expect(pending.result()).toBeNull();
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
});
});
// AUTISTMASK_TX_RESPONSE signs and broadcasts a transaction, so it is honoured
// only for a transaction approval. A reject shaped as this message used to
// retire a sign or connection approval outright, and an approve carrying a
// signed artifact used to run the broadcast path against an approval that names
// no transaction, failing closed only by throwing deeper in.
describe("a transaction response is honoured only for a transaction approval", () => {
test("a reject does not retire a sign approval", async () => {
const bg = loadBackground();
const pending = bg.requestSign();
await settle();
const id = pending.id();
bg.send(
{ type: "AUTISTMASK_TX_RESPONSE", id, approved: false },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toBeNull();
// Still live: its own reject settles it.
bg.send(
{ type: "AUTISTMASK_SIGN_RESPONSE", id, approved: false },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
});
test("a reject does not retire a connection approval", async () => {
const bg = loadBackground({ actionPopup: true });
const pending = bg.requestSite();
await settle();
const id = pending.id();
bg.send(
{ type: "AUTISTMASK_TX_RESPONSE", id, approved: false },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toBeNull();
// Still live: the port that owns it connects the site.
const port = bg.connectApproval(id);
port.decide(true, false);
port.disconnect();
await settle();
expect(pending.result()).toEqual({ result: [signer.address] });
});
test("an approve carrying a signed transaction never broadcasts against a sign approval", async () => {
const bg = loadBackground();
const pending = bg.requestSign();
await settle();
const id = pending.id();
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
expect(pending.result()).toBeNull();
});
});
// A handler that throws must still answer. `sendResponse` is the only thing
// that settles the page's window.ethereum.request() promise, so a throw that
// escapes a handler leaves that promise pending forever — no error, no
+59
View File
@@ -0,0 +1,59 @@
// Tests for the debug/testnet banner (issue #375).
//
// On a testnet the banner is raised even in a release build, but it must not
// append the active view's internal id: the user should see "[TESTNET]", never
// "[TESTNET] (approve-tx)". The suffix is gated on the compile-time DEBUG
// constant, which is false in a plain test load, so this drives exactly the
// text a shipped build renders. Revert the gate to the old unconditional
// suffix and this fails.
//
// The banner is created on demand by updateDebugBanner(); the document stub
// records what it prepends so the assertion can read the resulting text.
function makeBanner() {
return {
id: "",
textContent: "",
style: { cssText: "" },
remove() {},
};
}
function makeDocument() {
let banner = null;
return {
getElementById(id) {
return id === "debug-banner" ? banner : null;
},
createElement: () => makeBanner(),
body: {
prepend(node) {
banner = node;
},
},
};
}
function load() {
jest.resetModules();
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
globalThis.document = makeDocument();
const helpers = require("../src/popup/views/helpers");
const { state } = require("../src/shared/state");
return { helpers, state };
}
describe("the release banner on a testnet", () => {
test("carries no internal view id", () => {
const { helpers, state } = load();
state.networkId = "sepolia";
helpers.updateDebugBanner("approve-tx");
expect(
globalThis.document.getElementById("debug-banner").textContent,
).toBe("[TESTNET]");
});
});