Achhina's Digital Garden

2026-09-04 OpenRouter Interceptor Review Findings Actioned

5 min read

2026-09-04 OpenRouter Interceptor Review Findings Actioned

What I set out to do

A previous session had audited background.ts, the options page, the privilegedTabs experiment and the OpenRouter service and left seven findings. The question this session was whether they were real and worth doing. They were, so all seven got actioned, tests first.

What I actually did

Verified each finding against the code before touching anything. All seven held; one was overstated (the unused experiment methods were behind an allowlist, not open to arbitrary URLs).

The one that mattered. The WebP to PNG conversion script interpolated a page-sourced avatar URL straight into a template literal passed to tabs.executeScript. A single quote survives URL parsing in the path component, so a crafted img.src on the page could break out of the string and run in the content-script world. Now JSON.stringify. The API path had sanitizeAvatarPath; the DOM fallback never got the same treatment.

Domain filter before script injection. handleOpenRouterRequest injected the avatar-extraction script into the page and only then asked the domain filter whether the tab was excluded. Pulled the tab-description step out into src/background/capture-tab.ts as describeCaptureTab(tabId, settings, lookup): read the tab, run the filter, then extract avatars, returning null when the filter excludes. Six tests written first against an injected lookup, including the fail-open cases (no tab, unreadable tab) that the filter’s own rule already promised.

validateApiKey no longer lies. It caught every rejection and returned false, so a valid key on a dead connection read as “Invalid API key”. Introduced OpenRouterApiError carrying the status; only a 401 is now “false”, everything else rethrows, and the router reports it as “Could not check the API key: …” with the real message. erasableSyntaxOnly refused the parameter-property shorthand on the error class, which is the first time that flag has said anything to me.

Small ones. Toast exit animation named slideOutUp against a keyframe called slide-out-up, so the toast just vanished. deliverCard never revoked its blob URL while every sibling did after ten seconds. A debug onBeforeRequest listener on <all_urls> logged every request the browser made. openPrivilegedUrl and updateToPrivilegedUrl had no callers, so they went along with the allowlist that only existed to guard them; the schema copy under public/ and two doc mentions followed.

1280 unit tests green, typecheck, both lints and build clean.

Second pass, from a review of the fixes. Four tightenings, all taken.

  • The blob revoke in deliverCard was registered after the await, so a failed download still leaked. Rather than a try/finally in three places, one downloadBlob(downloads, blob, options) in src/background/download-blob.ts now owns create, download and delayed revoke for the state export, the card delivery and the devtools capture. Three tests, fake timers.
  • validateApiKey returns a verdict, accepted | rejected | unfunded, instead of boolean | throws. 401 is rejected, 402 is unfunded (“The API key works, but the account has no credits”), anything else still throws with the real message. The reviewer had 403 down as “key lacks model access”; OpenRouter’s error reference says 403 is moderation flagging the input, so it stays a thrown error rather than a key verdict.
  • The reviewer’s third point, that an unreadable tab still gets scripted, was already false for a tab tabs.get rejects on. It was true for a tab whose URL comes back undefined. Now an unknown URL passes the filter (the capture is kept) but does not earn an injection. The comment says why the two rules differ.
  • The PNG conversion script moved out of background.ts into buildAvatarPngExpr in src/utils/avatar-png-expr.ts, the same builder-plus-evalExprRaw pattern as the other injected expressions. Three tests: draws and resolves to bytes, rejects naming the URL, and a hostile URL with a quote and a statement in it is loaded verbatim as data and sets nothing on the global. That last one is the regression test the first pass could not write against an inline template. happy-dom’s canvas has getContext and toBlob with nothing behind them, which made them easy to stand in for.

1288 unit tests green, typecheck, both lints and build clean. Committed as two: 951a192 fix(background) for the fixes plus the PNG builder extraction, and 7ced73b refactor(experiments) for the privilegedTabs trim.

What was striking

The lint rule that bit was unbound-method, on expect(lookup.extractAvatarImages).not.toHaveBeenCalled(). The fix was in the interface, not the test: declaring the lookup’s members as function-typed properties instead of method signatures says what they are (injected callbacks, no this) and the rule stops asking.

Top 3 next

  1. Confirm in the live PWA that the toast now animates out and the console is quieter without the debug listener
  2. Test the key check against a real unfunded key to see the 402 message land on the options page
  3. Rebuild dist and reload the extension in the JAI profile so the PWA picks up the new background

Graph View

Backlinks