fix(build): flip web_accessible_resources to use_dynamic_url

[dmnyc]

Fixes #95 (L-14).

crxjs emits web_accessible_resources with use_dynamic_url: false, exposing stable chrome-extension:// URLs any site can probe to fingerprint the installed extension. A post-build Vite plugin now flips it to true so resource URLs are per-instance. Honored on Chromium 130+; older versions ignore the key and fall back to stable URLs (min version stays 116). Verified against a fresh build.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[dmnyc]

@claude review

[claude]

Claude finished @dmnyc's task in 4m 37s —— View job

---

Review

• Gather context (diff, audit doc, related files)
• Review vite.config.ts WAR plugin
• Review keystore.ts throttle change
• Review offscreen.ts sender gate
• Post review

Note: the branch carries three commits (0ceea6c offscreen, 7ac7f47 keystore, 1cc2bd8 WAR) — the PR body only describes the last one. Reviewed all three since they all land on main if this merges.

---

1. offscreen.ts sender gate doesn't close the path that's actually reachable — High

src/offscreen/offscreen.ts:18 gates on sender.id !== browser.runtime.id. Two problems:

a) The threat named in the comment can't reach this listener. Cross-extension messages fire runtime.onMessageExternal, never runtime.onMessage, so "a co-installed extension can message us directly by id" doesn't describe a path through this code. (That's also why the audit's M-2 recommendation said "optionally plus an origin check" — the origin check is the load-bearing half.)

b) The extension's own content scripts pass this gate, and they relay page-controlled payloads:

• src/content/content.ts:79-87 copies page-supplied params through, stripping only type and source — target and req survive.
• src/background/index.ts:2932 deliberately ignores target === "offscreen", so nothing else inspects the message.
• For a content-script sender, sender.id is this extension's id (sender.origin is the page's origin — which is exactly why the background router keys on origin at index.ts:2940, not id).

So any web page can do:

window.postMessage({
  source: "apogee-provider", id: "1", method: "rpc",
  params: { target: "offscreen", req: { kind: "broadcastPset", network: "liquid", pset: "<attacker pset>" } },
}, "*");

…and reach handle() directly, bypassing origin auth and the approval flow entirely. The offscreen doc's sendResponse is the only one for that message, so the engine's result is returned to the page too. Reachable ops are the ones that don't need a mnemonic (broadcastPset, sync/checkEsplora as a fetch channel, getAddress/getBalance for an attacker-supplied descriptor, qr, getAsset) — no key exfiltration, but this is a strictly wider surface than the co-installed-extension scenario the commit set out to close.

Suggested fix — mirror the router's actual discipline, plus belt-and-braces in the bridge:

// offscreen.ts — extension pages/SW only; content scripts carry the page's origin
const EXT_ORIGIN = new URL(browser.runtime.getURL("/")).origin;
if (sender.id !== browser.runtime.id || sender.origin !== EXT_ORIGIN) return false;

Worth a quick check that sender.origin is populated for the service-worker sender in your target Chrome versions; if (sender.tab) return false; is the equivalent-and-safer variant here (unlike the background router, nothing legitimately messages the offscreen doc from a tab). And in content.ts:82, drop target (and req) alongside type/source, so the bridge can't smuggle routing keys into any future listener.

Fix this →

2. WAR plugin swallows every failure, including the ones you'd want to hear about — Medium

vite.config.ts:42-46:

try { built = JSON.parse(fs.readFileSync(manifestPath, "utf8")); }
catch { return; } // no manifest in this build (e.g. playground) — nothing to do

The playground rationale doesn't hold — vite.playground.config.ts is a separate config that never loads this plugin, and apply: "build" already excludes dev. In practice the only way this catch fires is a crxjs output-path change or a malformed manifest — i.e. precisely the regressions that should fail the build rather than silently ship a manifest without the hardening. Narrow it to ENOENT and rethrow otherwise (or drop the try entirely and let it throw).

Same theme: nothing asserts the invariant afterwards. The provider-conformance job already builds dist/; a two-line check that every web_accessible_resources entry has use_dynamic_url === true would stop a crxjs upgrade from quietly undoing this.

Fix this →

3. Confirm the conformance suite is green on this PR — please verify

I couldn't build here (dependency install is blocked in this sandbox), so the plugin review is static. The risk worth confirming: crxjs registers content-script chunks in web_accessible_resources, and with use_dynamic_url: true the static chrome-extension://<id>/… URL stops resolving on Chromium versions that enforce it. If any crxjs-emitted loader hardcodes a static URL instead of resolving through chrome.runtime.getURL() at runtime, MAIN-world provider injection breaks on new Chrome while still passing on old — a failure mode manual smoke-testing on one browser can miss in either direction.

e2e/liquid-provider.spec.ts loads the real dist/ in a current Chromium and asserts the MAIN-world provider announces, so a green Provider conformance run on this PR is the evidence that matters. Worth stating in the PR body which Chromium version the "verified against a fresh build" check used.

4. Scope note on L-14 itself — Info

Fine to land, but let's not over-credit it: this extension declaratively injects world: "MAIN" provider code into <all_urls> and announces itself (manifest.config.ts content_scripts), which is a far louder presence signal than probing a WAR URL. The audit already calls this "cosmetic hardening" — the plugin doesn't change Apogee's fingerprintability in practice. No action, just don't let #95 read as "fingerprinting closed".

5. keystore.ts throttle — looks right

src/keystore/keystore.ts:345-356 now matches unlock() and verifyPassword() exactly: assertAttemptAllowed() → derive → recordUnlockFailure() on mismatch → clearUnlockFailures() on success. Correct fix for L-1, and the doc comment explains why. Two notes:

• changePassword still has no callers anywhere in src/ (matches the audit's "dead code today"). When it's wired up, the caller has to route the throw through unlockErrMessage() (src/sidepanel/errors.ts:42) or users see a raw UNLOCK_THROTTLED:1755… string.
• There's no keystore test file at all, so the throttle invariant across all three entry points is unguarded. Worth a small unit test now that three call sites must stay in sync.

Nits

• vite.config.ts:40 — path.resolve(outDir, …) resolves against process.cwd(), while Vite resolves build.outDir against config.root. Identical here (and a no-op if the resolved outDir is already absolute), but capturing config.root in configResolved makes it exact regardless.
• vite.config.ts:49 — the some(...) idempotency guard is a nice touch; it also means a manifest that legitimately has zero WAR entries never gets rewritten, which is the right behavior.

---

Summary: the keystore fix is clean. The WAR plugin does what it says with one meaningful robustness gap (#2) and one thing to confirm in CI (#3). The offscreen commit is the one I'd hold: as written it hardens against a path that can't reach the listener while leaving the page-driven path through the content bridge open (#1).
· fix/95-war-use-dynamic-url

[dmnyc]

The failing Provider conformance run is real, and the cause is the PR's own change. Reproduced locally with evidence:

1. Built this branch → dist/manifest.json has use_dynamic_url: true on both WAR entries → playwright test fails injects the safe public surface into an ordinary page (same provider-name timeout as CI), twice in a row on CI.
2. Same build, manifest hand-edited back to use_dynamic_url: false, nothing else touched → 2 passed (6.3s).

So use_dynamic_url: true breaks the crxjs content-script loader on current Chromium — exactly the failure mode the review's item 3 flagged. I've reverted the flip (9c09848); the branch now matches main and CI should go green.

Recommendation for #95: close as wontfix. The scan itself labels L-14 cosmetic, and the review confirms why — this extension declaratively injects a MAIN-world provider into <all_urls> and announces itself, which is a far stronger presence signal than probing a WAR URL; the flip bought no real fingerprinting protection and broke injection. Revisit only if crxjs gains first-class dynamic-URL support for content-script loaders. @dmnyc your call — say the word and I'll close the PR and the issue with this comment as the record.