v0.3.1: security audit fixes — trust boundary (ADR-016), detector hardening, chainId, L2 fail-secure#8
Merged
Merged
Conversation
## Feature — expandable Recent Activity cards Clicking a Recent Activity entry in the popup now expands it to show the data we already had in storage but weren't rendering: - Full address (copyable via browser selection, mono font) - Absolute timestamp (formatDate) - Site hostname (if a URL was recorded) - Blocked / Allowed status with icon - Threats list (human-formatted) Implementation: - `RecentScan` gains optional `threats`, `url`, `blocked` — the fields were already written to storage via `recordScan`; we just widen the popup's view of them - `RecentActivity.tsx`: semantic `<button class="activity-main">` for the summary row + sibling `.activity-details` block on expand - Module-level `expandedKey` signal so the expanded state persists across the signal-driven re-renders (and across closing / reopening the popup within the same session) - `expand_more` chevron rotates 180deg via CSS transition CSS: - `.activity-item` restructured into summary + details stack - Focus-visible outline on the clickable row (keyboard a11y) - Expanded state gets `border-radius: top-only` on summary to visually connect with details block Accessibility: - Uses `<button>` + `aria-expanded` (no `role="button"` hack) - Enter/Space naturally toggle via native button semantics ## Release — v0.3.0 bundle - `manifest.json`: version `0.2.0` -> `0.3.0`; refreshed description to reflect broader scope (not just EIP-7702) - `README.md`: detector count 14 -> 23, test counts to current (395 core / 375 extension), added replay-risk + phishing domain blocking + proxy resolution rows, architecture section now mentions fail-closed decision matrix (ADR-013) - `store-assets/testudo-extension.zip` repackaged Tests: 375 extension + 395 core pass. Build + lint clean.
Second-pass visual polish based on user feedback ("font size is very
small") and design-skill review. Aesthetic commitment: precision
security terminal — dense but scannable, color-keyed to risk.
Typography:
- Base 13px UI / 12px mono for values (was 9-11px)
- 10px caps tracked labels (was 9px)
- line-height 1.45 on details block for breathing room
Risk-colored accent rail:
- 2px bar at the top of the expanded block via ::before pseudo-element
- color-keyed via data-risk attribute (danger / warn / safe / dim)
- ties the details to the risk badge above without adding chrome
Status as a health pill, not an icon:
- Pill shape (border-radius 999) with color-bg + border
- Replaced Material icon with 6px dot + 3px halo via color-mix
- Halo always matches the text color (red-on-red for blocked,
teal-on-teal for allowed) — reads as a health indicator
Threats as terminal chips:
- UPPER_SNAKE_CASE preserved (no re-casing) for log-tag feel
- 10px Geist Mono, warn-palette (bg + border + text)
- flex-wrap row so multi-threat cases flow in the 360px popup width
Other refinements:
- Removed duplicate activity-status-icon rule
- Condensed padding and grid gaps (14/12/16 vs 12/8/8)
- Fixed :focus-visible with accent outline
Files:
- popup.html: ~80 lines of CSS refactored
- RecentActivity.tsx: chip-style threats, dot-based status, data-risk hook
- store-assets/testudo-extension.zip: repackaged
Build + lint clean, 375 tests still pass.
Swaps the README header icon from the rasterized PNG to the vector SVG source. Renders crisply at any resolution on GitHub (the PNG was 128x128 and got interpolated on retina).
Adds a tiny dotenv loader to rolldown.config.ts that reads `packages/extension/.env.local` (gitignored) on build. Explicit env vars still win — the file only sets keys that aren't already defined. Developers can now: cp .env.local.example .env.local # paste API key yarn build # picks it up automatically Previously every build required exporting TESTUDO_API_KEY in the shell, otherwise the built SW would log a recurring auth_error from pingApi() and show the `!` badge. No new dependency — 25-line parser handles KEY=value, quoted values, and #comments. `.env*.local` is already in .gitignore.
…sync Chrome was throwing `Uncaught ReferenceError: Cannot access 'c' before initialization` inside the injected script, locking the wrapped window.ethereum.request proxy and causing every analyze/check call to time out with "TESTUDO_ANALYZE_REQUEST timeout". Root cause: `const timer = setTimeout(...)` was declared AFTER the onResponse subscription, but the subscription's callback referenced `timer` inside `clearTimeout(timer)`. If the response listener fired synchronously (any sync reply path from content.ts — e.g. the invalid- address early-return, or cross-world CustomEvent dispatch racing inside the same tick), the callback hit a TDZ on the uninitialized `const`. Minified, `timer` became `c`, hence the cryptic error. The stack made it look like a channel bug, but the channel is fine — the request and response events are on distinct nonce-scoped names. Fix: hoist `let timer` above the subscription. Null-check it in the callback. Classic pattern for mutual-reference closures. Regression was latent (sync reply paths exist in content.ts for invalid addresses) but was exposed after S-16 shipped chainId through the payload — possibly because Chrome's cross-world CustomEvent dispatch tightened, or because some test-dapp flow now exercises the sync branch. Fix is TDZ-safe regardless of why sync reply happens. Tests still 375 passing. Build clean.
Pre-existing staged release prep: manifest 0.3.0->0.3.1, package.json 0.2.0->0.3.1 (reconcile drift), store-assets/testudo-v0.3.1.zip, and the v0.3.1 decision/roadmap entries. Note: this zip predates the AUDIT-1/2/3 security fixes that follow; it must be rebuilt before any Chrome Web Store resubmission (which is on hold pending Synpress E2E of the trust-boundary rework). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bypass (AUDIT-3) A bare PUSH1 0x01 (ubiquitous: loop counters, booleans, lengths) before a CALL/STATICCALL was treated as ecrecover signature-verification, setting hasAuthorizationPattern=true and suppressing every CRITICAL/HIGH !auth branch in analyzeTokenTransfers. An attacker could downgrade a hardcoded-destination token drainer from CRITICAL/blocked with one stray opcode. Now PUSH1 0x01 counts only when corroborated by a KECCAK256 in the same look-back window (real signature schemes hash the message before the precompile call); the unambiguous PUSH20 0x..01 precompile-address literal still stands alone. Fixtures that encoded the old assumption updated to realistic ecrecover shapes. 3 new tests (bypass guards + real-ecrecover guard); 398 core tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…16, AUDIT-1/2)
Replaces the ADR-011 nonce/CustomEvent channel, which had two critical bugs:
AUDIT-1 leaked the channel nonce to any page script (defeating forgery
protection), and AUDIT-2 let a page starve the one-shot handshake so
createChannel('') threw at module top level and disabled ALL interception.
A document-shared secret is never private between the ISOLATED and MAIN worlds.
Fix: the ISOLATED content script transfers a private MessagePort to MAIN once at
document_start (before any page script), so authority is an unforgeable
capability, not a secret. injected.js is now a declared world:MAIN content
script (more CSP-robust than DOM <script src>, which strict-CSP dApps blocked),
built as a classic IIFE; removed from web_accessible_resources. Order-resilient
handshake (ISO_READY/MAIN_READY); fail-secure async init (no top-level throw).
Validated by a Codex + Gemini dual-model brainstorm. 7 channel tests
(correlation, timeout, handshake wiring); 369 extension tests pass; build clean.
Phase 1 only — warning UI stays in MAIN. PENDING Synpress E2E (merge gate;
checklist in ADR-016) before shipping. Phase 2 (move verdict/UI to ISOLATED +
full fail-secure) tracked in ADR-016.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Multi-agent security audit (3 CRITICAL, 3 HIGH, 12 MEDIUM, 26 LOW) recorded in BUGS.md with file:line + fixes and per-finding status. DECISIONS.md + ROADMAP.md record the ADR-016 trust-boundary v2 rework, the AUDIT-3 core fix, and the remaining open HIGH/MEDIUM/LOW work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
parseChainIdHex always used base 16, so a typed-data chainId serialized as a
decimal string ("137") became 311 and per-chain threat lookups were routed to a
non-existent chain — a silent false negative on L2/L3. Extracted a pure,
unit-tested parseChainId (utils/chain-id.ts, 8 tests): 0x-prefixed -> base 16,
otherwise base 10; also handles number/bigint and rejects 0/negative/non-integer.
Wired through injected.tsx (getCurrentChainId, chainChanged, permit/typed-data/
EIP-7702 call sites). Resolves AUDIT-11 (lookup/display now agree), AUDIT-30
(now tested), mitigates AUDIT-29 (only positive integers reach the chain URL).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…UDIT-5/26) Bytecode + deployer analysis was hardcoded to mainnet RPC + mainnet Blockscout but ran for every chain, so on an L2/L3 it queried the WRONG chain — a false "clean" for an L2-only contract, or a bogus verdict from an unrelated mainnet contract at the same address. Gate the local layers to mainnet (chainId undefined|1); off-mainnet, skip them and return local UNKNOWN, which the decision matrix preserves (never a false clean) while the chain-aware threat API still decides. deployerCache now keyed by chainId (AUDIT-26). 4 new tests. Follow-up: per-chain RPC + explorers to actually analyze L2/L3 (needs endpoint + host_permissions decisions). See DECISIONS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ute AUDIT-6 AUDIT-6 alleged the bloom filter dropped ~99.97% of phishing lookups due to an insert/query bitCount mismatch across serialization. Reproduced and REFUTED: the constructor derives bitCount = bits.length*8 for BOTH insert and query, so the moduli always match. New test builds 2000 domains at 250K (non-byte-aligned) sizing, round-trips through base64, and asserts zero false negatives. No source change — guards the invariant against future regressions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BUGS.md status: AUDIT-4/5 fixed, AUDIT-6 refuted, AUDIT-11/26/29/30 resolved. ROADMAP completed entry. DECISIONS: off-mainnet analysis fails secure (full per-chain RPC deferred as follow-up). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tate) origin/main advanced with the squash-merge of the v0.3.0 PR (#7), which our branch already contains as individual commits plus the v0.3.1 + security rework. Resolved all three conflicts to ours (the newer, intended state): - manifest.json -> v0.3.1 + MAIN-world injected.js content script (ADR-016) - services/messaging.ts -> MessagePort bridge (ADR-016), not the old nonce channel - store-assets/testudo-extension.zip -> kept deleted (renamed to testudo-v0.3.1.zip) Verified merged tree: build clean, 382 extension tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e739281b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "matches": ["<all_urls>"], | ||
| "js": ["injected.js"], | ||
| "run_at": "document_start", | ||
| "world": "MAIN", |
There was a problem hiding this comment.
The newly committed packages/extension/store-assets/testudo-v0.3.1.zip is not built from this manifest: inspecting the archive shows its manifest.json still has only the isolated content.js content script and still exposes injected.js as a web-accessible resource, and its bundled content.js still contains the old testudo-handshake/DOM injection path. If that zip is uploaded to the Chrome Web Store, none of the ADR-016 MessagePort/MAIN-world bridge changes in this commit will ship, so the release continues using the old trust-boundary code despite the source fix.
Useful? React with 👍 / 👎.
Lykhoyda
added a commit
that referenced
this pull request
Jun 14, 2026
…ex PR #8 P1) The committed testudo-v0.3.1.zip was built BEFORE the ADR-016 security fixes: its manifest still has only the isolated content.js, exposes injected.js as a web-accessible resource, and its content.js still has the old testudo-handshake DOM-injection path — so uploading it would ship none of the MessagePort/MAIN-world work. Remove it and gitignore store-assets/*.zip; the release workflow builds the zip from current source in CI. Rebuild before any Chrome Web Store resubmission. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ex PR #8 P1) The committed testudo-v0.3.1.zip was built BEFORE the ADR-016 security fixes: its manifest still has only the isolated content.js, exposes injected.js as a web-accessible resource, and its content.js still has the old testudo-handshake DOM-injection path — so uploading it would ship none of the MessagePort/MAIN-world work. Remove it and gitignore store-assets/*.zip; the release workflow builds the zip from current source in CI. Rebuild before any Chrome Web Store resubmission. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Synpress e2e-docker gate passed on 298cabb (the trust-boundary work), so integrating the cleanup PRs (#9-#12) now merged to main. Conflicts resolved: - .gitignore — kept BOTH the screenshots/ (from #9) and store-assets/*.zip (from #8) ignore entries. - docs/DECISIONS.md (git-crypt) — kept the #12 Dependabot Remediation entry AND the #8 ADR-016 / off-mainnet-fail-secure entries. Verified merged tree: build clean, 398 core + 382 extension tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ships v0.3.1 plus the fixes from a multi-agent security audit of the extension +
@testudo/core. All 3 CRITICAL and 3 HIGH findings are resolved (one HIGH was a false positive); MEDIUM/LOW remain tracked indocs/BUGS.md.CRITICAL — fixed
73b9b3d, ADR-016): replaced the ADR-011 nonce/CustomEvent handshake — which leaked the channel nonce to any page script and could be starved into throwing at module top-level, disabling all interception — with a private MessagePort bridge + a declaredworld:"MAIN"content script. Order-resilient handshake, fail-secure async init (no top-level throw), classic IIFE bundle,injected.jsremoved fromweb_accessible_resources.4e22936):detectEcrecoverno longer treats a strayPUSH1 0x01as a signature-verification auth pattern (which let an attacker suppress CRITICAL drainer warnings with one opcode). Now requires a corroboratingKECCAK256in the look-back window, or the unambiguousPUSH20 0x..01precompile-address literal.HIGH — addressed
8042890): extracted a pure, unit-testedparseChainId(0x-hex vs decimal). A decimal-stringchainIdlike"137"no longer becomes 311 → no more wrong-chain threat lookups. Also resolves AUDIT-11 (lookup/display agree) and AUDIT-30 (now tested); mitigates AUDIT-29.7df2c20): off-mainnet, bytecode + deployer analysis was silently running against mainnet RPC. Now fails secure — skipped off-mainnet → localUNKNOWN(never a wrong-chain verdict); the chain-aware threat API still decides. Resolves AUDIT-26 (chainId-keyed deployer cache). Follow-up: per-chain RPC + explorers to actually analyze L2/L3 (needs endpoint +host_permissionsdecisions — seedocs/DECISIONS.md).2ea695a): ❌ refuted as a false positive. The constructor setsbitCount = bits.length*8for both insert and query, so the moduli always match. Reproduced with a 2000-domain / 250K-sized (non-byte-aligned) round-trip showing zero false negatives; regression guard added, no source change.Verification
Follow-ups (tracked in docs)
isTrusted-gated overrides) and flip risky-method handling to full fail-secure on bridge-down. Closes UI-spoofing/clickjacking that Phase 1 doesn't.docs/BUGS.md.ADRs
🤖 Generated with Claude Code