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

New Issue

clawbot · 2026-08-17T10:11:24+02:00

clawbot commented

2026-08-17 10:11:24 +02:00

The repo's stated Next Step, and the last thing standing between milestone 1.0.0 and a tag. Milestone 1.0.0 is complete (61 closed, 0 open) and #190 is merge-ready, but no security review of the extension as a whole has been done — only the individual defects that were filed and fixed one at a time.

This is an audit task, not a code-change unit. Its deliverable is findings. Every finding gets its own issue with its own definition of done; this issue is closed when the audit is complete and its findings are filed, not when they are fixed.

Why it is worth doing as a whole rather than as its parts

Every unit that landed in 1.0.0 was reviewed against its own definition of done. Nothing has looked at the assembled system for defects that live between units — a guard that is correct in isolation but reachable around, an invariant two units each half-enforce, a path that only exists once several changes coexist. The approval machinery alone was reworked by #271, #275 and #280 in the same session.

Scope

Three lanes, as named in TODO.md:

Key and secret handling — key material at rest and in memory; encrypt/decrypt paths; what crosses the extension messaging boundary; what reaches the DOM and when it is cleared; what survives a view exit, a popup close, or a worker restart; what a compromised content script or page can reach.
DEBUG-mode policy and build integrity — that DEBUG cannot be on in a shipped bundle, that verify-build cannot be satisfied by a bundle that did not come from this tree, and that no debug-only path leaks secrets or bypasses a gate when enabled.
RPC and page input validation — every value arriving from a dApp, a page, an RPC response or the blocklist artifact. Type confusion, missing bounds, values trusted because a sibling field was checked, and anything where a malformed input reaches signing or display.

Rules for the audit

Adversarial, not confirmatory. The output that matters is what is wrong. Do not restate what is right beyond what is needed to show the search was real.
Every finding must be reproduced, not theorised — a failing test, a probe, or a concrete input-to-outcome path. A finding that cannot be demonstrated is filed as a question, labelled as such.
Severity is stated in terms of consequence, not adjectives: what an attacker gets, what the user loses, and what has to be true first.
Do not fix anything. Do not open a PR. Findings only.

Definition of done

All three lanes audited, with the method and coverage stated — including what was NOT examined and why.
Each finding filed as its own issue, with reproduction, consequence, preconditions, and an acceptable fix.
Findings that turn out to be non-issues are recorded as such rather than silently dropped, so the same ground is not re-covered.
A summary comment on this issue listing every filed finding by number, and an explicit statement of whether anything found should block the 1.0.0 tag.
No code change in this unit; make check untouched and still green.

The repo's stated Next Step, and the last thing standing between milestone 1.0.0 and a tag. Milestone 1.0.0 is complete (61 closed, 0 open) and https://git.eeqj.de/sneak/AutistMask/pulls/190 is merge-ready, but no security review of the extension as a whole has been done — only the individual defects that were filed and fixed one at a time. This is an **audit task, not a code-change unit.** Its deliverable is findings. Every finding gets its own issue with its own definition of done; this issue is closed when the audit is complete and its findings are filed, not when they are fixed. ## Why it is worth doing as a whole rather than as its parts Every unit that landed in 1.0.0 was reviewed against its own definition of done. Nothing has looked at the assembled system for defects that live *between* units — a guard that is correct in isolation but reachable around, an invariant two units each half-enforce, a path that only exists once several changes coexist. The approval machinery alone was reworked by https://git.eeqj.de/sneak/AutistMask/issues/271, https://git.eeqj.de/sneak/AutistMask/issues/275 and https://git.eeqj.de/sneak/AutistMask/issues/280 in the same session. ## Scope Three lanes, as named in `TODO.md`: 1. **Key and secret handling** — key material at rest and in memory; encrypt/decrypt paths; what crosses the extension messaging boundary; what reaches the DOM and when it is cleared; what survives a view exit, a popup close, or a worker restart; what a compromised content script or page can reach. 2. **DEBUG-mode policy and build integrity** — that `DEBUG` cannot be on in a shipped bundle, that `verify-build` cannot be satisfied by a bundle that did not come from this tree, and that no debug-only path leaks secrets or bypasses a gate when enabled. 3. **RPC and page input validation** — every value arriving from a dApp, a page, an RPC response or the blocklist artifact. Type confusion, missing bounds, values trusted because a sibling field was checked, and anything where a malformed input reaches signing or display. ## Rules for the audit - **Adversarial, not confirmatory.** The output that matters is what is wrong. Do not restate what is right beyond what is needed to show the search was real. - **Every finding must be reproduced**, not theorised — a failing test, a probe, or a concrete input-to-outcome path. A finding that cannot be demonstrated is filed as a question, labelled as such. - **Severity is stated in terms of consequence**, not adjectives: what an attacker gets, what the user loses, and what has to be true first. - Do not fix anything. Do not open a PR. Findings only. ## Definition of done - [ ] All three lanes audited, with the method and coverage stated — including what was NOT examined and why. - [ ] Each finding filed as its own issue, with reproduction, consequence, preconditions, and an acceptable fix. - [ ] Findings that turn out to be non-issues are recorded as such rather than silently dropped, so the same ground is not re-covered. - [ ] A summary comment on this issue listing every filed finding by number, and an explicit statement of whether anything found should block the 1.0.0 tag. - [ ] No code change in this unit; `make check` untouched and still green.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#303