Work on any URL, not just gov.bb domains (via activeTab) - #7
Open
TaigaTi wants to merge 2 commits into
Open
Conversation
Host access was pinned to *.gov.bb plus localhost/127.0.0.1 by decision 0003, which existed to clear the Chrome Web Store's "Broad Host Permissions" flag. That allowlist no longer matches how the tool is used: prototypes live on Netlify/Vercel/preview URLs, vendor forms live on vendor domains, and local dev is not only localhost. Off-allowlist the extension did nothing and said nothing, because the toast is drawn by the content script that could not be injected. - manifest: host_permissions and content_scripts[].matches -> <all_urls>. crxjs mirrors the matches onto the generated web_accessible_resources, so the content script's dynamic-import chunk is reachable from every origin too — required, or the ensureContentScript fallback injects a loader that cannot import its own payload. permissions stay ["scripting", "storage"]; activeTab stays dropped as <all_urls> already authorises executeScript. 1.0.4 -> 1.1.0. - src/shared/urlSupport.ts: report why a page cannot be filled. Two checks, because Chrome withholds tab.url on pages we have no access to — describeUnsupportedUrl reads the URL up front, describeInjectionFailure translates the executeScript rejection (and names the "Allow access to file URLs" toggle for file: URLs). An unknown URL does not block a fill. - background: pass tab.url through both entry points; unwrap errors with errorMessage so the popup shows the reason, not "Error: …". - docs: decision 0006 accepts the Web Store trade-off and records the justification to submit; 0003 marked superseded; plan + summary added; README/PRIVACY/ORG_DISTRIBUTION permission wording corrected. No fill logic changed — none of it was domain-aware. The Barbados-flavoured values are chosen from field labels, so they behave identically on any host. Verified: 237/237 tests pass (207 existing + 30 new), pnpm build clean, and dist/manifest.json confirms <all_urls> in all three host fields at 1.1.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
The first cut of this change simply widened both manifest fields to <all_urls>.
That works, and it hands back exactly the Chrome Web Store "Broad Host
Permissions" flag decision 0003 existed to remove — plus the "read and change
all your data on all websites" install warning, for a tool that only ever
touches one tab after an explicit click.
activeTab grants one tab, any origin, on a user gesture, and Chrome grants it
for both entry points here: invoking the action (which opens the popup) and
firing a `commands` shortcut. Same reach, no flag, smaller install prompt, and
nothing injected into pages the user never invokes it on — stricter than the old
allowlist, which injected into every .gov.bb page on load.
- manifest: permissions ["activeTab", "scripting", "storage"]; host_permissions
and content_scripts keys removed entirely. Built manifest now has no
web_accessible_resources either, and `<all_urls>` appears nowhere in dist/.
- vite.config.ts: the content script had to leave the manifest, because crxjs
wraps a declared one in a loader whose dynamic-imported payload gets a
generated web_accessible_resources entry — the third place the broad pattern
appeared. executeScript({files}) needs no web-accessible entry but does need
one flat file, so a second lib/IIFE pass emits a self-contained
dist/content.js (21.5 kB, zero imports). Chained off the main build's
closeBundle so `pnpm build` and `pnpm dev` stay single commands.
- background: inject CONTENT_SCRIPT_FILE rather than reading a hashed name from
getManifest().content_scripts. The ask-first/inject-on-silence order now also
prevents a repeat fill registering a duplicate onMessage listener.
- docs: 0006 rewritten around activeTab, recording the deferred
optional_host_permissions follow-up (it would restore pre-injected fills and
reach cross-origin iframes, which activeTab cannot); plan/summary updated;
README, PRIVACY and ORG_DISTRIBUTION corrected, including a pre-existing
stale claim that the content script matches <all_urls>.
Verified: 237/237 tests pass; clean-dist build emits both passes; built manifest
asserts activeTab-only with no host_permissions/content_scripts/WAR keys; and
content.js contains no import/__vitePreload/getURL, with its onMessage listener
inside the injected IIFE.
Co-Authored-By: Claude Opus 5 <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
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.
Form Filler now fills forms on any page you open it on — prototype hosts, preview URLs, vendor forms,
localhost, localfile://pages — instead of only*.gov.bbandlocalhost/127.0.0.1, while asking for fewer permissions than before.Why
Host access was pinned to the
.gov.bballowlist by decision 0003, which existed to clear the Chrome Web Store's "Broad Host Permissions" flag. That allowlist stopped matching how the tool is used: prototypes are served from Netlify/Vercel/GitHub Pages/preview URLs, vendor forms live on vendor domains, and local dev is not onlylocalhost(LAN IPs,*.local, tunnels,file://). Off-allowlist the extension did nothing and said nothing — the toast is drawn by the very content script that could not be injected.activeTab, not<all_urls>The first commit here simply widened both manifest fields to
<all_urls>. That works, and it hands back exactly the flag decision 0003 existed to remove — plus the "read and change all your data on all websites" install warning, for a tool that only ever touches one tab after an explicit click. The second commit replaces it:activeTabgrants one tab, any origin, on a user gesture — and Chrome grants it for both entry points this extension has: invoking the action (which opens the popup) and firing acommandsshortcut. Same reach, no Broad Host Permissions flag, no broad install warning, and nothing injected into pages you never invoke it on — stricter than the old allowlist, which injected into every.gov.bbpage on load.The build change this required
The content script had to leave the manifest. crxjs wraps a declared content script in a loader that dynamic-imports its payload, and that chunk gets a generated
web_accessible_resourcesentry whosematchesis the third place the broad pattern appears — the mechanism decision 0003 documented. Declaring nothing removes all three.chrome.scripting.executeScript({ files })reads extension files directly and needs no web-accessible entry, but it does need one flat file. Sovite.config.tsgained a second lib/IIFE pass emitting a self-containeddist/content.js(21.5 kB, zero imports — it only pulls infieldExtractorandtoast; faker lives in the background), chained off the main build'scloseBundlesopnpm buildandpnpm devstay single commands.runFillstill messages the tab before injecting. That used to be an optimisation for pre-existing tabs; now that nothing is pre-injected it also stops a repeat fill from registering a duplicateonMessagelistener.Telling the user why a page can't be filled
src/shared/urlSupport.ts(new) — "works anywhere" means users will press Alt+Shift+F onchrome://extensionsand the Web Store. Two checks, because one is not enough:describeUnsupportedUrlreads the tab URL up front (chrome://,devtools://,chrome-extension://,about:,view-source:,data:, the Web Store).describeInjectionFailuretranslates theexecuteScriptrejection — the only signal for pages whose URL Chrome withholds from the extension (achrome://tab often reports none). It names the "Allow access to file URLs" toggle when afile:injection is refused.An unknown URL deliberately returns
nulland lets the fill proceed, rather than blocking a page we merely cannot read.runFillfailures are unwrapped witherrorMessage, so the popup shows "Form Filler can't run on this page — …" instead ofError: …. Decision 0003 listed this message as "possible future work"; running everywhere is what made it necessary.No fill logic changed: none of it was domain-aware. The Barbados-flavoured values (phone numbers,
BBxxxxxpostcodes, TAMIS references) are chosen from field labels, so they behave identically on any host.Trade-offs to know about
activeTabgrants the tab's top-level origin, not third-party frames. The code never passedallFrames, so nothing is lost, but a form inside a cross-origin iframe can't be reached this way.CONTENT_SCRIPT_FILE↔vite.config.ts). Renaming one side breaks injection silently, and no test imports the built file — only a browser load catches it.optional_host_permissionswas deliberately deferred. An opt-in<all_urls>grant would restore pre-injected fills and reach cross-origin frames, and optional permissions sit outside the broad-host review. It needs opt-in UI and a second code path; decision 0006 records it as the follow-up if either limitation bites.Docs
Decision 0006 added and 0003 marked superseded (its goal survives — only the allowlist is gone). Plan + summary per repo convention.
README.md,PRIVACY.mdanddocs/ORG_DISTRIBUTION.mdpermission wording corrected — they still listedactiveTabandtabsfrom before those were removed, and ORG_DISTRIBUTION still claimed the content script matches<all_urls>and needs a broad-permission justification.Verification
pnpm test— 237/237 pass: 207 pre-existing unchanged, plus 30 new intests/urlSupport.test.ts(allowed schemes, every blocked scheme, both Web Store hosts, unknown/absent URLs, injection-failure translation).distpnpm buildruns both passes; asserted in the built output:permissions: ["activeTab","scripting","storage"], nohost_permissions/content_scripts/web_accessible_resourceskeys,<all_urls>absent from every file indist/, anddist/content.jsfree ofimport/__vitePreload/getURLwith itsonMessage.addListenerinside the injected IIFE.dist/unpacked and check: a fill on a non-.gov.bbform; two fills on one page (no duplicate-listener effects); afile://prototype with and without the file-access toggle; achrome://page showing its reason in the popup. Worth doing before any store submission, since no test exercises the built file.🤖 Generated with Claude Code