security: fix high-severity findings from audit (closes #6) #7

clawbot · 2026-02-27T20:38:42+01:00

clawbot commented

2026-02-27 20:38:42 +01:00

Fixes from the security review (issue #6):

Unpredictable approval IDs — replaced sequential nextApprovalId++ with crypto.randomUUID()
Origin validation — derive RPC origin from trusted sender.origin / sender.tab.url instead of trusting msg.origin from content script
Popup message sender validation — approval/response messages now verified to come from extension URL only
eth_sign danger warning — prominent red warning for raw hash signing requests
Secret zeroing — best-effort clearing of decrypted secrets after use
Password min length 8→12 — ⚠️ NOTE: this is a policy change not specified in RULES.md, flagged for sneak to approve or revert
TODO comments for password plaintext over runtime.sendMessage (future improvement)

make check passes (after yarn install).

Closes #6

Fixes from the security review (issue #6): 1. **Unpredictable approval IDs** — replaced sequential `nextApprovalId++` with `crypto.randomUUID()` 2. **Origin validation** — derive RPC origin from trusted `sender.origin` / `sender.tab.url` instead of trusting `msg.origin` from content script 3. **Popup message sender validation** — approval/response messages now verified to come from extension URL only 4. **eth_sign danger warning** — prominent red warning for raw hash signing requests 5. **Secret zeroing** — best-effort clearing of decrypted secrets after use 6. **Password min length 8→12** — ⚠️ NOTE: this is a policy change not specified in RULES.md, flagged for sneak to approve or revert 7. **TODO comments** for password plaintext over runtime.sendMessage (future improvement) `make check` passes (after `yarn install`). Closes #6

sneak was assigned by clawbot

2026-02-27 20:38:42 +01:00

clawbot added 7 commits 2026-02-27 20:38:42 +01:00

security: increase minimum password length from 8 to 12 characters 1237cf8491

security: replace predictable sequential approval IDs with crypto.randomUUID() 95314ff229

security: add prominent danger warning for eth_sign requests 13e2bdb0b0

security: derive RPC origin from sender instead of trusting msg.origin d59ebfd461

security: validate sender URL for popup-only messages b478d9efa9

security: add TODO comments for password plaintext over runtime.sendMessage f13cd0fd47

security: clear decrypted secrets after use (best-effort)

check / check (push) Successful in 21s

Details

eec96f9054

clawbot commented

2026-02-27 20:38:52 +01:00

Code Review

LGTM with one flag:

✅ No RULES.md / linter / Makefile changes
✅ crypto.randomUUID() — correct fix for predictable approval IDs
✅ Origin derivation from sender.origin / sender.tab.url — proper defense against content script origin spoofing
✅ Popup sender validation using runtime.getURL() — prevents external pages from sending approval responses
✅ eth_sign warning — appropriate, this is a dangerous method
✅ Secret zeroing — best-effort is the right approach for JS
✅ make check passes

⚠️ Flag: Password minimum changed 8→12 (commits in addWallet.js and importKey.js). RULES.md does not specify a minimum. This is a security policy decision for sneak to approve or revert.

## Code Review **LGTM with one flag:** ✅ No RULES.md / linter / Makefile changes ✅ `crypto.randomUUID()` — correct fix for predictable approval IDs ✅ Origin derivation from `sender.origin` / `sender.tab.url` — proper defense against content script origin spoofing ✅ Popup sender validation using `runtime.getURL()` — prevents external pages from sending approval responses ✅ eth_sign warning — appropriate, this is a dangerous method ✅ Secret zeroing — best-effort is the right approach for JS ✅ `make check` passes ⚠️ **Flag**: Password minimum changed 8→12 (commits in addWallet.js and importKey.js). RULES.md does not specify a minimum. This is a security policy decision for sneak to approve or revert.

clawbot referenced this pull request

2026-02-27 20:43:31 +01:00

fix: low-severity security findings L3, L4, L5 (closes #6) #8

sneak added 1 commit 2026-02-27 20:56:11 +01:00

Merge branch 'main' into fix/high-severity-security

check / check (push) Successful in 21s

Details

b01df0639b

sneak merged commit 0413c52229 into main

2026-02-27 20:56:43 +01:00

sneak deleted branch fix/high-severity-security

2026-02-27 20:56:43 +01:00