Commit Graph

9 Commits

Author SHA1 Message Date
clawbot
82c40e009d test: delete the unreachable seal() hook and guard non-object RPC bodies
All checks were successful
check / check (push) Successful in 24s
Three review findings, no redesign.

seal() was installed after session.close(), so onLate could never fire:
once the context is destroyed no route handler and no console listener
exists to record anything. It was dead code that the attribution table,
run.js and harness.js all described as the final safety net. Deleted
rather than moved — take() already drains everything the collector holds
before teardown, so it covered nothing, and this harness must not ship a
mechanism it cannot demonstrate. The now-unreachable `late > 0` term in
the failure condition goes with it.

handleRpc() parsed `postData || "null"` and then dereferenced the result,
so a POST whose body Playwright reports as null — a bodyless request, or
any payload it cannot decode as UTF-8, e.g. sendBeacon with a Blob —
threw a TypeError inside the route handler and killed node mid-suite:
truncated TAP, no summary, no failure line. Non-object payloads now take
the same path as unparseable ones and are reported as unstubbed traffic,
which is the entire point of that branch.

README claimed unrecognised outbound requests are reported as failures,
unqualified, while observation in fact ends TRAILING_WATCH_MS after the
last test returns. The limit is now stated where it lands.
2026-08-09 16:26:26 +00:00
clawbot
a13862d991 test: make e2e error attribution total, and fix the README canary text
All checks were successful
check / check (push) Successful in 19s
The error collector had a window API (mark/since) and twice a record fell
outside somebody's window and was silently dropped, producing a green run
that proved nothing: first the mark started after test 1, discarding
everything recorded during launch; then the tail after the final test was
never read at all, so a request escaping the fixtures at the end of the
last test reported 5/5 passed and exit 0.

Rather than patch a second boundary and invite a third, the window
concept is gone. ErrorCollector exposes only take(), which always drains
everything outstanding, so successive takes partition the whole record
stream with no gaps, and seal(), which closes the stream at the end of
the run and routes stragglers straight to a failure. Attribution is
total by construction: launch through test 1 goes to test 1, each
subsequent interval to the test that ends it, the tail to the suite.

The tail also needs to exist before it can be drained. A request a test
fires without awaiting reaches the route handler about 10ms after that
test's function resolves, and closing the context does not wait for it,
so with no window at all it died unobserved. The run now keeps
collecting for a bounded 1.5s after the last test before teardown.

Also:

- README described a canary that was built, found to kill the service
  worker, and deleted. Replaced with what actually runs: the harness
  waits for the background worker's own startup blocklist fetch to reach
  the route handler and aborts if it does not. Documents
  --host-resolver-rules=MAP * ~NOTFOUND as defence in depth.
- The canary's failure message asserted traffic was escaping to the real
  internet and blamed the -e flag. It cannot distinguish that from a lost
  startup race, so it now states what was observed and lists both causes.
- E2E_TRACE_NETWORK was compared strictly to "1", so E2E_TRACE_NETWORK=true
  silently did nothing. Recognised on/off values are accepted and anything
  else is a hard error rather than a quiet default.
- The measured margin that makes the canary sound is route install at
  11-23ms against the worker fetch at 525-883ms, not the 30s timeout
  slack the comment cited.
2026-08-09 15:50:45 +00:00
clawbot
a3075f2f47 test: intercept the MV3 service worker's network in the e2e harness
All checks were successful
check / check (push) Successful in 43s
ctx.route() does not see requests made by the background service worker
unless Playwright is run with PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1,
so the phishing blocklist fetch that src/background/index.js issues at
worker startup was reaching raw.githubusercontent.com on the real
internet on every run. phishingDomains.js swallows fetch failures, so
nothing surfaced it, and the raw.githubusercontent.com stub in
tests/e2e/network.js was unreachable code that made the gap look covered.

script/test-e2e now sets the flag, with a comment recording what to do if
a future Playwright drops it. The flag being experimental is not taken on
trust: launch() waits for the worker's own startup request to arrive in
the route handler and refuses to run the suite if it never does, so
escaping traffic fails the run instead of passing unnoticed. Chrome is
additionally started with --host-resolver-rules=MAP * ~NOTFOUND, so
anything that does slip past interception cannot reach a real host.

Also: errors and unstubbed requests recorded during launch are attributed
to the first test rather than discarded, a suite that registers no tests
now fails instead of exiting 0, a failure after the browser is up tears
the context down instead of hanging the process, E2E_TRACE_NETWORK=1
prints every routed request tagged [sw] or [page], and the dead exports in
harness.js and network.js are gone.
2026-08-09 15:11:19 +00:00
d89629d090 test: containerized Chrome end-to-end harness that drives the real popup (closes #181)
All checks were successful
check / check (push) Successful in 55s
`make check` was green while the AddToken screen crashed on every open.
`script/lint` is only `prettier --check`, so a used-but-not-imported
identifier is invisible until a browser evaluates it. This adds a suite
that runs the real popup in a real Chrome and treats any uncaught page
error or console.error as a failure.

- `script/test-e2e` (with `make test-e2e` as a thin shim) builds
  `dist/chrome/` and runs `tests/e2e/run.js` inside the Playwright image,
  pinned by digest. `playwright-core` is pinned to the matching 1.56.0
  through `yarn.lock`; the two must be bumped together because the
  browsers ship inside the image.
- Deliberately outside `script/test` and `script/check`: REPO_POLICIES
  caps `make test` at 20 seconds. Nothing under `tests/e2e/` is named
  `*.test.js`, so jest cannot pick it up either.
- Launches with `channel: "chromium"`; the default headless shell
  silently refuses to load extensions with no error at all. The extension
  id is read from the service worker URL, never hardcoded.
- All http(s) traffic is intercepted at the browser level and served from
  fixtures, so the run is deterministic and offline. Unrecognised
  outbound requests are reported as failures rather than allowed.
- A missing build or an unavailable container fails loudly; a skip that
  looks like a pass is the failure mode this is meant to prevent.
- One allowlisted page error, for the libsodium WASM CSP fallback tracked
  as #182, which is otherwise untouched here.

The suite was demonstrated failing against the unfixed tree with
`pageerror: showView is not defined` and `pageerror: addressDotHtml is
not defined`, so it carries the two one-line import fixes it caught:

closes #150 — `showView` restored to the destructure in
`src/popup/views/addToken.js`, dropped by a22f33d, which made the
AddToken screen unreachable and corrupted the navigation stack.

closes #151 — `addressDotHtml` restored in
`src/popup/views/transactionDetail.js`, dropped by df031fd, which threw
before `showView("transaction")` for every ERC-20 transfer. The shared
`renderAddressHtml` helper is not used here on purpose: it hardcodes the
`/address/` explorer URL, and this row needs the token-specific `/token/`
link.
2026-08-09 14:30:25 +00:00
f7f141a757 security: make DEBUG a build-time flag defaulting to off (closes #149) (#169)
Some checks failed
check / check (push) Has been cancelled
Fixes the highest-severity item in the repo: `src/shared/constants.js` had
`const DEBUG = true;`, so `generateMnemonic()` returned the publicly committed
`DEBUG_MNEMONIC` for every wallet created from a build of `main`, and the real
entropy path was dead code in every artifact we could produce.

## What changed

**`build.js`** — `AUTISTMASK_DEBUG` is read from the environment and injected
as a `__BUILD_DEBUG__` entry in the existing esbuild `define` map, next to the
other `__BUILD_*__` defines. Only the exact value `1` enables it; unset, empty,
`true`, or a typo all yield a release build, so the insecure direction requires
a deliberate opt-in and any mistake fails safe. The build prints
`Build mode: release (DEBUG off)` or
`Build mode: DEBUG (INSECURE - hardcoded test mnemonic, do not ship)`.

**`src/shared/constants.js`** — `DEBUG` now uses the same `typeof` guard that
`src/shared/buildInfo.js` already uses for the other build-time defines, and
defaults to `false` when the define is absent (jest, plain `require`).
`DEBUG_MNEMONIC` stays in the tree and stays exported.

**`Makefile`** — new `build-debug` target (`AUTISTMASK_DEBUG=1` + the same
build) so a debug build stays a one-liner for development.

**`README.md`** — new "Debug Builds" subsection under Getting Started, and the
DEBUG Mode Policy section now states that `DEBUG` is build-time-only and spells
out the boundary against the runtime toggle.

**`tests/wallet.test.js`** — new, covering both build modes.

**`TODO.md`** — refreshed in the same commit (details at the bottom).

No new `if (DEBUG)` branch was added and nothing about what DEBUG *does* changed:
still exactly the red banner plus the hardcoded test phrase, per the README
DEBUG Mode Policy and `RULES.md:76-80`.

## The interaction with the #145 settings toggle

This is the subtle part, so spelling out the reasoning.

There are two distinct debug flags in the tree after #145:

1. the compile-time `DEBUG` constant from `constants.js`, and
2. the runtime `debugMode` state flag, which the settings easter egg toggles
   and which `settings.js:379` pushes into `log.js` via `setRuntimeDebug()`.

`log.js` merges them: `isDebug()` is `DEBUG || _runtimeDebug`. That merged
value feeds exactly two things — the log level threshold (`log.js:24`) and the
red banner (`views/helpers.js:71`). Making the banner user-toggleable is the
intended behavior of #145, and this PR leaves it alone.

`generateMnemonic()` does **not** consult `isDebug()`. It reads the
compile-time `DEBUG` binding directly. That distinction is what makes a release
build coherent: with `__BUILD_DEBUG__` false, `DEBUG` is false in the bundle,
so no amount of clicking the version ten times and flipping the toggle can
reach `return DEBUG_MNEMONIC`. The user can turn the banner and verbose logging
on in a release build; they cannot turn the hardcoded phrase on.

The failure mode to guard against is someone later "tidying up" the two flags by
routing `wallet.js` through `isDebug()`, which would silently reintroduce this
exact vulnerability with the runtime toggle as the trigger. Three things now
guard that: a comment at the `wallet.js` call site saying it must stay the
compile-time constant and why, the same statement in the README DEBUG Mode
Policy, and a regression test that calls `setRuntimeDebug(true)`, asserts
`isDebug()` is genuinely true, and then asserts `generateMnemonic()` still
returns fresh entropy.

I considered instead making the runtime toggle unavailable in release builds,
but rejected it: that removes a feature #145 deliberately added, and it defends
the wrong boundary. The banner is not the dangerous part; the mnemonic path is,
and that one is already unreachable.

## Verification

`make check` — green, 5 suites, 55 tests, plus lint and fmt-check. It also ran
via the pre-commit hook on the commit itself.

The new tests, per the verification standard in the manager comment on the
issue (not just `a !== b`) — with the flag off: two successive
`generateMnemonic()` calls differ, both pass `isValidMnemonic`, both are 12
words, neither equals `DEBUG_MNEMONIC`, and the result derives a usable HD
wallet (`xpub` + a well-formed first address), so a broken implementation
returning a counter or a truncated phrase would fail. Same assertions again
with the runtime toggle forced on. With the flag on (`__BUILD_DEBUG__` defined
before a `jest.resetModules()` re-require): `DEBUG` is `true` and
`generateMnemonic()` returns `DEBUG_MNEMONIC`, so the debug path is proven
working rather than silently deleted.

Build artifacts — `make build` and `make build-debug` both produce `dist/chrome`
and `dist/firefox` successfully. Grepping the minified bundles for the emitted
`DEBUG` export value across all four bundles (chrome popup, chrome background,
firefox popup, firefox background):

    # after make build
    $ grep -roh 'DEBUG:![01]' dist/chrome dist/firefox | sort | uniq -c
          4 DEBUG:!1

    # after make build-debug
    $ grep -roh 'DEBUG:![01]' dist/chrome dist/firefox | sort | uniq -c
          4 DEBUG:!0

`!1` is minified `false`, `!0` is `true`. Also checked the fail-safe path:
`AUTISTMASK_DEBUG=true make build` prints `Build mode: release (DEBUG off)` and
likewise yields `4 DEBUG:!1`.

One thing a reviewer should know about the grep: the `DEBUG_MNEMONIC` string
literal is still present in the release bundle. That is not a leak of anything
(the phrase is in this public repo already) and it does not mean the branch is
live — esbuild cannot tree-shake a CommonJS `module.exports` object, so the
constant survives while `DEBUG` folds to `false`. The compiled function is
`function PL(){return ML?UL:f_.fromEntropy(globalThis.crypto.getRandomValues(new Uint8Array(16))).phrase}`
where `ML` is the `DEBUG:!1` export. So "the phrase string is absent" is *not*
the right test for a release build; "the exported `DEBUG` is `!1`" is, which is
what I checked.

## `TODO.md` refresh

Per the manager comment: Status rewritten (no branch in flight —
`feat/issue-144-settings-about` landed as #145, scripts-to-rule-them-all landed
as #148, so the `scripts/` question is resolved; `make check` recorded as
verified green on `main` at `23aeae4`); the completed "Verify main passes make
check" Future Step removed; Future Steps rewritten against the #149-#168
backlog in rough priority order, keeping branch pruning (now #167) and the
pre-1.0 security review (noting #149 and #157 are parts of it but it is
broader).

One deliberate deviation to flag rather than bury: the manager asked that Next
Step become this issue. Taken literally against the Workflow section, this
commit *completes* #149, which would normally move it into Completed Steps. I
followed the repo's existing convention for in-flight work instead — the
previous Next Step was phrased as "Land feat/issue-144-settings-about", so Next
Step is now "Land #149 ... PR open, awaiting review", which is accurate until
this merges. Whoever merges should move it to Completed Steps and promote the
first Future Step. Happy to change it if the reviewer prefers the strict
reading.

## Out of scope

`script/lint` being `prettier --check` only and unable to catch undefined
identifiers (#152) — noted in the TODO but not fixed here; I greped for `DEBUG`
consumers by hand rather than relying on lint, as advised. Nothing else in the
DEBUG consumer set (`log.js`, `helpers.js`, `state.js`, `settings.js`) changed
behavior.

Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #169
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-08-09 16:19:09 +02:00
d35bfb7d23 feat: expand confirm-tx warnings — closes #114 (#118)
All checks were successful
check / check (push) Successful in 5s
Expands the confirm-tx warning system with three new warning types, all using the existing `visibility:hidden/visible` pattern from PR #98 (no animations, no layout shift).

## Changes

1. **Scam address list expanded** (7 → 652 addresses): Sourced from [MyEtherWallet/ethereum-lists](https://github.com/MyEtherWallet/ethereum-lists) darklist (MIT license). Checked synchronously before sending.

2. **Contract address warning**: When the recipient is a smart contract (detected via `getCode`), shows a warning that sending directly to a contract may result in permanent loss of funds.

3. **Null/burn address warning**: Detects known burn addresses (`0x0000...0000`, `0x...dead`, `0x...deadbeef`) and warns that funds are permanently destroyed.

4. **No-history warning** (existing from #98): Unchanged, still shows for EOAs with zero transaction history.

All warnings use reserved-space `visibility:hidden/visible` elements — no layout shift, no animations.

closes #114

Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Co-authored-by: user <user@Mac.lan guest wan>
Co-authored-by: clawbot <clawbot@eeqj.de>
Reviewed-on: #118
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>
Co-committed-by: clawbot <sneak+clawbot@sneak.cloud>
2026-03-01 19:34:54 +01:00
clawbot
3c2d553070 test: add V4 swap ERC20→ERC20 decoder regression test
All checks were successful
check / check (push) Successful in 37s
Adds a test that constructs a Uniswap V4 USDT→USDC swap using
SETTLE/SWAP_EXACT_IN_SINGLE/TAKE sub-actions inside a V4_SWAP command.
Without decodeV4Swap(), the output token would be unresolvable and the
swap name would not show 'USDT → USDC'. This test fails on the old code
and passes with the decodeV4Swap() fix.

Refs: #59
2026-02-28 11:22:14 -08:00
9e45c75d29 Implement personal_sign and eth_signTypedData_v4 message signing
All checks were successful
check / check (push) Successful in 4s
Replace stub error handlers with full approval flow for personal_sign,
eth_sign, eth_signTypedData_v4, and eth_signTypedData. Uses toolbar
popup only (no fallback window) and keeps sign approvals pending across
popup close/reopen cycles so the user can respond via the toolbar icon.
2026-02-27 15:27:14 +07:00
065f0eaa81 Add project scaffolding
All checks were successful
check / check (push) Successful in 10s
Makefile, Dockerfile, CI workflow, prettier config, manifests for
Chrome (MV3) and Firefox (MV2), source directory structure, and
minimal test suite. All checks pass.
2026-02-24 09:48:21 +07:00