chore: escaping hygiene left after #307 — a dead un-encoded explorer helper, two inconsistent bare interpolations, a UTF-16 slice #329

New Issue

clawbot · 2026-08-20T13:47:53+02:00

clawbot commented

2026-08-20 13:47:53 +02:00

Found by the review of #327 (#327 (comment)). Each was traced and is safe today; all four are traps for the next change rather than defects.

explorerLink(network, type, value) at src/shared/networks.js:47 is an exported, un-encoded twin of the new explorerUrl() helper. Dead before and after #307, so not a regression — but it undercuts the "one helper that percent-encodes" invariant #307 established, and the next caller who finds it by name gets the unsafe one. Delete it, or make it delegate.
src/popup/views/addressToken.js:202 — ${tokenDecimals} is the only value-shaped bare interpolation left, against the rule #307 writes at the top of helpers.js. Every writer was traced (parseInt/Number, no dApp-writable path into trackedTokens), so it is safe; it is also the one exception a reader will cite as precedent.
src/popup/views/home.js:267 leaves addrTotal bare while src/popup/views/deleteAddress.js:96 escapes the identical formatAddressTotal() output. Both safe, mutually inconsistent — one of them is teaching the wrong lesson.
displaySymbol() slices at a UTF-16 boundary — a 20-emoji symbol truncates onto a lone \ud83d and renders U+FFFD. Cosmetic, but the cap is right next to a security control and should not look sloppy.

Definition of done

explorerLink() is deleted or delegates to explorerUrl(); no un-encoded URL builder remains exported.
Items 2 and 3 escape consistently, so src/popup/views/ has no bare interpolation into an innerHTML string — making the grep in #307's DoD a clean invariant with no exceptions to remember.
displaySymbol() truncates on grapheme or code-point boundaries, never mid-surrogate. Test with an astral-plane symbol.
make check green.

Found by the review of https://git.eeqj.de/sneak/AutistMask/pulls/327 (https://git.eeqj.de/sneak/AutistMask/pulls/327#issuecomment-67567). Each was traced and is **safe today**; all four are traps for the next change rather than defects. 1. **`explorerLink(network, type, value)` at `src/shared/networks.js:47`** is an exported, un-encoded twin of the new `explorerUrl()` helper. Dead before and after #307, so not a regression — but it undercuts the "one helper that percent-encodes" invariant #307 established, and the next caller who finds it by name gets the unsafe one. Delete it, or make it delegate. 2. **`src/popup/views/addressToken.js:202`** — `${tokenDecimals}` is the only value-shaped bare interpolation left, against the rule #307 writes at the top of `helpers.js`. Every writer was traced (`parseInt`/`Number`, no dApp-writable path into `trackedTokens`), so it is safe; it is also the one exception a reader will cite as precedent. 3. **`src/popup/views/home.js:267`** leaves `addrTotal` bare while `src/popup/views/deleteAddress.js:96` escapes the identical `formatAddressTotal()` output. Both safe, mutually inconsistent — one of them is teaching the wrong lesson. 4. **`displaySymbol()` slices at a UTF-16 boundary** — a 20-emoji symbol truncates onto a lone `\ud83d` and renders U+FFFD. Cosmetic, but the cap is right next to a security control and should not look sloppy. ## Definition of done - [ ] `explorerLink()` is deleted or delegates to `explorerUrl()`; no un-encoded URL builder remains exported. - [ ] Items 2 and 3 escape consistently, so `src/popup/views/` has **no** bare interpolation into an `innerHTML` string — making the grep in #307's DoD a clean invariant with no exceptions to remember. - [ ] `displaySymbol()` truncates on grapheme or code-point boundaries, never mid-surrogate. Test with an astral-plane symbol. - [ ] `make check` green.

clawbot referenced this issue

2026-08-20 14:26:18 +02:00

pre-1.0 security review: key handling, DEBUG-mode policy, RPC input validation #303

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#329