From 66ef95e80a332606f0f3072563f784772f738d65 Mon Sep 17 00:00:00 2001 From: Rendy Wijaya <65700377+rendyuwu@users.noreply.github.com> Date: Tue, 6 Oct 2026 10:17:00 +0700 Subject: [PATCH 1/3] feat(extension): open the popup from the inline picker's more-logins row (#21) When the page's host has no entries of its own but other hosts of the domain do, the picker's last row now opens the toolbar popup through the service worker (`inline-open-popup`, `chrome.action.openPopup`). It fills nothing and releases nothing, so it needs a trusted click but not the six guards. Where the popup cannot open, the footer turns into text naming the Extensions menu and the `fill-login` shortcut for the rest of the page. Closes #20. --- ARCHITECTURE.md | 2 +- CHANGELOG.md | 6 ++ KNOWN-LIMITS.md | 1 + README.md | 20 +++++ SECURITY.md | 6 ++ extension/src/background.ts | 38 +++++--- extension/src/content/inline.ts | 39 +++++++-- extension/src/lib/messages.ts | 9 +- extension/test/inline.spec.ts | 130 +++++++++++++++++++++++++--- extension/test/playwright.config.ts | 4 +- 10 files changed, 220 insertions(+), 35 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6898fe7..1255dc3 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -113,7 +113,7 @@ App data dir of the bundle id (`dev.rendy.subclave`; `pnpm tauri:dev` uses 3. **Lock**: manual, from the lock button, the status bar or the `vault.lock` shortcut (`useVaultStore.lock`, `vault_lock`, `lock_inner`); idle, when `spawn_vault_tick_thread` calls `VaultState::access` every 5 s past the deadline that `vault_touch` extends on activity (`drain_auto_lock`); minimize, in `on_main_window_event` when `lock_on_minimize` is set; tray, from the Lock item in `tray.rs`. Every path calls `Unlocked::wipe` and drops the key, then `emit_locked` closes the sync session (`session_closed`), drops every browser connection (`close_all`) and emits `subclave:vault-locked` with the `LockReason`. 4. **Sync pull and push**: `startSync` builds the scheduler (`createPolicy`). It pulls on unlock and on window focus (`onFocus`, at most once per `FOCUS_INTERVAL_MS`), and pushes `PUSH_DEBOUNCE_MS` after the first change of a burst (`markDirty`, on any `subclave:vault-changed` whose origin is not `sync`). Passes run one at a time (`serialize`) and re-read the switch each time (`readConfig`). Pull: `sync_pull` snapshots `locals_from_payload` and the etags, releases the lock, runs `pull` (list, fetch the objects whose etag moved, `open_envelope`, `merge`, prune this device's expired tombstones), lands the report with `apply_pull` (`merge_into_payload` per record) and `commit`s, then emits origin `sync`. Push: `sync_push` takes `take_dirty_envelopes`, `push` seals each one (`seal_envelope`) and writes it conditionally on its etag where the backend can, re-merging once on a conflict (`retry_merged`), then `finish_push` clears the marks that landed and `commit`s. 5. **Browser fill**: every request goes service worker, `createNativeTransport`, `subclave-proxy` (`read_frame`, `write_frame`), then the app's `accept_loop`, `handle_conn` and `dispatch` (status, locked check, `hello` and `auth` handshake, then the action). - - Inline: `startInline` and `createInline` draw an icon per field found by `scan`. Opening the picker sends `inline-logins`; the service worker answers it in `handleInline` at `scope: "host"`, and fills and the picker use the sender's own URL (`getState`, `get-logins`, `get_logins`, which returns no password). A pick passes `guard.check` (`createGuard`), then `inline-fill` runs `fillEntry` with `via: "inline"`: `get-credential` (`get_credential` re-runs `matching::matches` and requires `same_host`), then `subclave:fill` to the content script, where `fillTarget` refuses another origin and `fillCredential` fills. + - Inline: `startInline` and `createInline` draw an icon per field found by `scan`. Opening the picker sends `inline-logins`; the service worker answers it in `handleInline` at `scope: "host"`, and fills and the picker use the sender's own URL (`getState`, `get-logins`, `get_logins`, which returns no password). A pick passes `guard.check` (`createGuard`), then `inline-fill` runs `fillEntry` with `via: "inline"`: `get-credential` (`get_credential` re-runs `matching::matches` and requires `same_host`), then `subclave:fill` to the content script, where `fillTarget` refuses another origin and `fillCredential` fills. When `get_logins` counts `otherMatches`, the picker's last row sends `inline-open-popup`, which runs `openPopup` (`chrome.action.openPopup`) and nothing else. Where that is missing or rejects, the answer carries the `fill-login` shortcut from `chrome.commands.getAll`, and the row turns into text naming the Extensions menu and the shortcut for the rest of the page (`popupFallback`). - Save prompt: `pass` wires the form of each login field (`wireForm`). A trusted submit, or a click on its submit button, that follows a user gesture reads the pair with `submittedLogin` and sends `inline-submitted`; the service worker keeps it per tab in `chrome.storage.session` with the sender's URL and a fresh id. Each page load in that tab, and the submitting page itself once the password has left its form, sends `inline-pending-save`, which runs `check-login` with the asking page's URL as `pageUrl` (`check_login`: `other-site` off the submit's site by the Domain rule, else the submitted host's exact-host entries, no password returned); a new or changed login opens the picker as a prompt in the viewport's corner (`promptSave`). Add or Update pass `guard.check`, then `inline-save` carries the pair's id and sends `save-login` with `via: "inline"` (`save_login` requires `same_host` for an update, and the old password goes to history); Cancel sends `inline-save-cancel`, which drops the pair. A new submit closes a prompt that is showing. - Popup: `PopupApp` asks `get-state` for the active tab at `scope: "all"`, and a pick sends `fill-entry`, which runs `fillEntry` with `via: "popup"` (`ensureContentScript` injects `content.js` when it is missing). - Command: `fill-login` runs `runFillCommand`: `get-logins` for the active tab, then one match fills with `via: "command"`, and several open the popup where `chrome.action.openPopup` exists, or fill the most recently used one. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f92889..655b660 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ The latest release only. Every earlier version: [GitHub Releases](https://github.com/rendyuwu/subclave/releases). Versions: [SemVer](https://semver.org/); before `1.0` a minor bump may break things. +## [Unreleased] + +### Changed + +- The inline picker's "more logins" footer is now a row that opens the Subclave popup, which lists the logins saved for other hosts of the domain. Where the browser cannot open the popup from the page, the footer names the Extensions menu and the fill shortcut instead. + ## [0.1.1] - 06-10-2026 No change to the desktop app or to the extension's code. diff --git a/KNOWN-LIMITS.md b/KNOWN-LIMITS.md index 0a2380f..1b4872c 100644 --- a/KNOWN-LIMITS.md +++ b/KNOWN-LIMITS.md @@ -29,6 +29,7 @@ lives, and what would change it. - **No fill in cross-origin iframes**: the content script runs in the top frame only and reaches same-origin iframes through `contentDocument`, which a cross-origin frame does not expose, so a login form inside one is neither detected nor filled. `extension/src/content/fill.ts` (`rootsOf`). Changes when: the content script runs in every frame with a per-frame origin check. - **The inline guards are JavaScript against JavaScript**: the six clickjacking guards run in the same page as the code they guard against, and Firefox has no `IntersectionObserver` v2, so the visibility guard (guard 6) runs on Chromium only. `extension/src/content/guard.ts` (`createGuard`). Changes when: browsers give extension UI a visibility guarantee. - **No inline UI without the Popover API**: Firefox 115 to 124 (and Chrome before 114) get no icon in login fields and no save prompt; the popup and the fill command still work. The save prompt is part of the inline UI, so "Show in login fields" off turns it off too. `extension/src/content/inline.ts` (`startInline`). Changes when: the minimum Firefox version reaches 125. +- **The more-logins row needs action.openPopup**: Chrome 118 to 126 open the popup only for a policy-installed extension and older Chrome not at all, and Firefox before 149 opens it only inside a user action handler (whether a click relayed from the page counts there is not tested). On those browsers the row's first click on a page turns it into text pointing at the Extensions menu and the fill shortcut for the rest of that page, and the next page shows the row again. `extension/src/background.ts` (`openPopup`), `extension/src/content/inline.ts` (`popupFallback` in `createInline`). Changes when: the minimum versions reach Chrome 127 and Firefox 149, or the service worker learns before the first click that the popup cannot open. - **Very late login forms get no icon**: a page with no login field 5 seconds after it has loaded stops watching for good, so a single-page app that shows its login form later (or navigates to it without a page load) gets no icon. The popup and the fill command still fill it. `extension/src/content/inline.ts` (`SETTLE_AFTER_LOAD_MS`, `settle` in `createInline`). Changes when: detection keeps a cheap watch running on pages without a field. - **An unhinted first login step gets no icon**: a text field on its own (the first step of a two-step login) gets an icon only when it says `autocomplete="username"` or `"email"`, which keeps icons out of search boxes and newsletter fields. A first step without the hint (Yahoo and Zoom when last checked) gets no icon; the popup and the fill command still fill it. `extension/src/content/detect.ts` (`scan`). Changes when: detection gets a name or label heuristic that still leaves search and newsletter fields alone. - **"Show in login fields" applies from the next page load**: the content script reads the setting once when the page loads, so open tabs keep their current state. `extension/src/content/index.ts` (`askSettings`). Changes when: the service worker tells open tabs about the change. diff --git a/README.md b/README.md index b5bf207..a7237f6 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,26 @@ Pair from the Extensions menu as in step 4 above. If the popup offers Pair with Subclave again after a restart, pair again and revoke the old entry under Settings > Browser. +## Fill a login + +Click a login field, or the key icon in it, to list the logins saved for that +page's exact host, and pick one. + +Logins for another host of the same domain (an entry for `github.com` opened +from `gist.github.com`) are not listed. The picker's last row, +`N more logins on github.com`, opens the Subclave popup, which lists every +match. Where the browser cannot open the popup from the page, the row turns +into text; then open Subclave from the browser's Extensions menu (or the +toolbar once pinned) and pick the login there. + +`Ctrl+Shift+L` (`Cmd+Shift+L` on macOS) fills the current page without the +picker: one match fills right away; several open the popup, or fill the most +recently used login where the browser cannot open it. + +The browser leaves the shortcut unset when another extension already holds +it. Set it at `chrome://extensions/shortcuts`, or in Firefox under +`about:addons`, gear menu, Manage Extension Shortcuts. + ## Build from source Needs Rust stable, Node 20.19+ with pnpm, and diff --git a/SECURITY.md b/SECURITY.md index 7d28951..768907a 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -54,6 +54,12 @@ Until `1.0.0`, only the latest minor gets security fixes. `example.com` login, which was the main vector in Marek Tóth's DEF CON 33 research ([DOM-based Extension Clickjacking](https://marektoth.com/blog/dom-based-extension-clickjacking)). + When other hosts of the domain hold matches, the picker's last row counts + them and only opens the popup (`chrome.action.openPopup`). It fills + nothing and releases nothing, so it needs a trusted click but not the six + guards. A clickjacked click on it can only open the popup, where a fill + still takes a click in browser-owned UI. Where the popup cannot be opened, + the row turns into text. - Inline fill needs a trusted user action and passes the six guards in `extension/src/content/guard.ts`. If any guard fails, nothing is filled. - The page never supplies the URL. It comes from the browser: diff --git a/extension/src/background.ts b/extension/src/background.ts index 0322c71..d4a687f 100644 --- a/extension/src/background.ts +++ b/extension/src/background.ts @@ -81,6 +81,7 @@ const INLINE_REQUEST_TYPES: Record = { "inline-pending-save": true, "inline-save": true, "inline-save-cancel": true, + "inline-open-popup": true, }; export type BackgroundOptions = { @@ -413,18 +414,7 @@ export function startBackground(options: BackgroundOptions = {}): void { return { type: "fill", ok: true, filled: 0 }; } const entries = logins.result.entries; - // `chrome.action.openPopup` is Chrome 127+; Firefox and older Chrome have - // no such member, so it is read through a named interface. - const action: { openPopup?: () => Promise } = chrome.action; - if (entries.length > 1 && action.openPopup) { - try { - await action.openPopup.call(chrome.action); - return { type: "fill", ok: true, filled: 0 }; - } catch { - // Firefox and older Chrome reject openPopup; fall through to the - // newest-match fallback below. - } - } + if (entries.length > 1 && (await openPopup())) return { type: "fill", ok: true, filled: 0 }; let pick = entries[0]; if (entries.length > 1) { for (const entry of entries) { @@ -579,7 +569,8 @@ export function startBackground(options: BackgroundOptions = {}): void { * URL, so they never reach another host's logins or the popup's wider match. * The save prompt saves to the browser-stamped URL of the tab's own submitted * sign-in, and shows only on a page of that sign-in's site (`check-login` - * decides, by the vault's Domain rule). */ + * decides, by the vault's Domain rule). `inline-open-popup` uses neither the + * tab nor the URL; it only opens the popup. */ const handleInline = async ( message: InlineRequest, tabId: number, @@ -622,6 +613,12 @@ export function startBackground(options: BackgroundOptions = {}): void { case "inline-save-cancel": await dropPending(tabId, message.id); return { type: "save", ok: true }; + case "inline-open-popup": { + if (await openPopup()) return { type: "popup", ok: true }; + const commands = await chrome.commands.getAll(); + const shortcut = commands.find((command) => command.name === "fill-login")?.shortcut; + return { type: "popup", ok: false, shortcut: shortcut ?? "" }; + } } }; @@ -683,6 +680,21 @@ function runtimeMessageType(message: unknown): string | null { return typeof type === "string" ? type : null; } +/** Opens the toolbar popup in the focused window. `chrome.action.openPopup` + * is open to every extension from Chrome 127 and needs no user action from + * Firefox 149; older versions lack it or reject the call, which reads as + * `false`. Read through a named interface because the member can be missing. */ +async function openPopup(): Promise { + const action: { openPopup?: () => Promise } = chrome.action; + if (!action.openPopup) return false; + try { + await action.openPopup.call(chrome.action); + return true; + } catch { + return false; + } +} + /** Chromium's UA always says "Chrome", so the other products are checked first. */ function browserName(): string { const ua = navigator.userAgent; diff --git a/extension/src/content/inline.ts b/extension/src/content/inline.ts index 23d516d..152c1d2 100644 --- a/extension/src/content/inline.ts +++ b/extension/src/content/inline.ts @@ -245,6 +245,13 @@ function createInline(): Inline { * to update. The prompt is the picker open with no anchor. */ let saving: { prompt: SavePrompt; listing: boolean } | null = null; let submitSeq = 0; + /** The fill command's shortcut ("" when unbound) once the service worker + * could not open the popup for this page; the more-logins footer is text + * from then on. */ + // ponytail: remembered per page, so a browser that can never open the popup + // shows the row until its first click on each page; have the service worker + // learn it up front if that matters. + let popupFallback: string | null = null; const isOpen = (): boolean => picker.matches(":popover-open"); @@ -456,6 +463,21 @@ function createInline(): Inline { } }; + /** Opens the toolbar popup, which lists the wider matches in browser-owned + * UI. Fills and releases nothing, so its row needs a trusted event only. */ + const openPopup = async (state: Extract): Promise => { + const mine = seq; + const response = await ask({ type: "inline-open-popup" }); + if (!response || mine !== seq) return; + if (response.type === "popup" && response.ok) close(); + else if (response.type === "popup") { + popupFallback = response.shortcut; + showReady(state); + } else if (response.type === "error") { + errorView(response.code, response.message); + } + }; + const showReady = (state: Extract): void => { const next: Row[] = []; if (anchor && fields.get(anchor) === true) { @@ -484,11 +506,18 @@ function createInline(): Inline { if (state.entries.length === 0) lines.push(note(`No logins for ${state.host}.`)); const more = state.otherMatches; if (more > 0) { - lines.push( - note( - `${more} more login${more === 1 ? "" : "s"} on ${state.domain}: use the Subclave toolbar button`, - ), - ); + const count = `${more} more login${more === 1 ? "" : "s"} on ${state.domain}`; + if (popupFallback === null) { + next.push({ + label: count, + detail: "Open in Subclave popup", + guarded: false, + run: () => openPopup(state), + }); + } else { + const press = popupFallback ? `, or press ${popupFallback}` : ""; + lines.push(note(`${count}: open Subclave from the browser's Extensions menu${press}`)); + } } render(next, lines); }; diff --git a/extension/src/lib/messages.ts b/extension/src/lib/messages.ts index 6acc0c3..db6ac8e 100644 --- a/extension/src/lib/messages.ts +++ b/extension/src/lib/messages.ts @@ -26,7 +26,9 @@ export type PopupRequest = * `inline-save-cancel`) act on the sign-in that same tab submitted: it saves * to the `sender.url` stamped on its `inline-submitted`, and shows only on a * page of that URL's site (`check-login` decides). `id` names the pending - * sign-in the prompt shows, so a newer one is never written in its place. */ + * sign-in the prompt shows, so a newer one is never written in its place. + * `inline-open-popup` only opens the toolbar popup, which lists the wider + * matches for the active tab in browser-owned UI; it names no entry and no URL. */ export type InlineRequest = | { type: "inline-settings" } | { type: "inline-logins" } @@ -37,7 +39,8 @@ export type InlineRequest = | { type: "inline-submitted"; username: string; password: string } | { type: "inline-pending-save" } | { type: "inline-save"; id: string; entryId: string | null } - | { type: "inline-save-cancel"; id: string }; + | { type: "inline-save-cancel"; id: string } + | { type: "inline-open-popup" }; /** A sign-in waiting for the save prompt. `id` is the pending sign-in's own; * `host` is the submitted page's hostname. */ @@ -71,6 +74,8 @@ export type SwResponse = | { type: "save-prompt"; prompt: SavePrompt | null } | { type: "save"; ok: true } | { type: "save"; ok: false; code: NmErrorCode; message: string } + | { type: "popup"; ok: true } + | { type: "popup"; ok: false; shortcut: string } | { type: "error"; code: NmErrorCode; message: string }; /** `url` is the page the credential was released for, and `anchored` says the diff --git a/extension/test/inline.spec.ts b/extension/test/inline.spec.ts index 2dd10d5..cb2b7ed 100644 --- a/extension/test/inline.spec.ts +++ b/extension/test/inline.spec.ts @@ -62,6 +62,48 @@ async function openOn(page: Page, ui: Inline, field: Locator): Promise { await waitForOptions(page, ui); } +/** Replaces `chrome.action.openPopup` in the service worker with a stub that + * counts its calls in `__popupOpens` and, with `reject`, rejects like Firefox + * before 149. `delete` already works on this property, so it is configurable. */ +async function stubOpenPopup(reject: boolean): Promise { + await harness.worker.evaluate((fail) => { + // The worker's global keeps the count between `evaluate` calls; this stub + // is the only writer, so the shape is known. + const counter = globalThis as unknown as { __popupOpens: number }; + counter.__popupOpens = 0; + Object.defineProperty(chrome.action, "openPopup", { + configurable: true, + value: async () => { + counter.__popupOpens += 1; + if (fail) throw new Error("openPopup rejected"); + }, + }); + }, reject); +} + +async function popupOpens(): Promise { + return harness.worker.evaluate(() => { + // Written only by `stubOpenPopup`; absent before the first stub. + const counter = globalThis as unknown as { __popupOpens?: number }; + return counter.__popupOpens ?? 0; + }); +} + +/** Replaces `chrome.commands.getAll` in the service worker, since headless + * Chromium leaves `fill-login` unbound. Another command comes first, so the + * lookup has to pick `fill-login` by name. */ +async function stubShortcut(shortcut: string): Promise { + await harness.worker.evaluate((key) => { + Object.defineProperty(chrome.commands, "getAll", { + configurable: true, + value: async () => [ + { name: "_execute_action", shortcut: "Alt+Shift+S" }, + { name: "fill-login", shortcut: key }, + ], + }); + }, shortcut); +} + const GUARD_FIXTURES = [ "guard-untrusted.html", "guard-timing.html", @@ -174,7 +216,7 @@ test("opens on a field click, on the icon and on ArrowDown, for this host only", await username.click(); await expect.poll(() => ui.popoverOpen()).toBe(true); - await expect.poll(() => ui.count(OPTION)).toBe(FAKE_ENTRIES.length); + await expect.poll(() => ui.count(OPTION)).toBe(FAKE_ENTRIES.length + 1); const text = await ui.pickerText(); for (const entry of FAKE_ENTRIES) { expect(text).toContain(entry.title); @@ -202,24 +244,86 @@ test("opens on a field click, on the icon and on ArrowDown, for this host only", expect(await page.evaluate(() => document.activeElement?.id)).toBe("username"); }); -test("the footer counts the logins left for the toolbar button", async ({}, testInfo) => { +test("the more-logins row opens the popup by pointer and by keyboard, and ignores an untrusted click", async ({}, testInfo) => { harness = await startHarness(testInfo); + await stubOpenPopup(false); await setMode(harness, "no-fields"); const page = await openFixture(harness, "login.html"); const ui = inlineOf(page); - await page.locator("#username").click(); - await expect.poll(() => ui.pickerText()).toContain("No logins for 127.0.0.1."); - expect(await ui.pickerText()).toContain( - `1 more login on ${FAKE_DOMAIN}: use the Subclave toolbar button`, - ); + await openOn(page, ui, page.locator("#username")); + expect(await ui.count(OPTION)).toBe(1); + const text = await ui.pickerText(); + expect(text).toContain(`1 more login on ${FAKE_DOMAIN}`); + expect(text).toContain("Open in Subclave popup"); + expect(text).toContain("No logins for 127.0.0.1."); + + // Not a guarded row: an untrusted click is ignored, not refused. + await ui.dispatchUntrustedClick(OPTION, 0); + await page.waitForTimeout(300); + expect(await popupOpens()).toBe(0); + expect(await ui.count(OPTION)).toBe(1); + expect(await ui.count(".message")).toBe(0); + + await clickInline(page, ui, OPTION, 0); + await expect.poll(() => popupOpens()).toBe(1); + await expect.poll(() => ui.popoverOpen()).toBe(false); await setMode(harness, "ok"); - const second = await openFixture(harness, "login.html"); - const secondUi = inlineOf(second); - await second.locator("#username").click(); - await expect - .poll(() => secondUi.pickerText()) - .toContain(`2 more logins on ${FAKE_DOMAIN}: use the Subclave toolbar button`); + const keyboard = await openFixture(harness, "login.html"); + const keyboardUi = inlineOf(keyboard); + await keyboard.locator("#username").focus(); + await keyboard.keyboard.press("ArrowDown"); + await waitForOptions(keyboard, keyboardUi); + expect(await keyboardUi.count(OPTION)).toBe(FAKE_ENTRIES.length + 1); + expect(await keyboardUi.pickerText()).toContain(`2 more logins on ${FAKE_DOMAIN}`); + for (let step = 0; step < FAKE_ENTRIES.length; step += 1) { + await keyboard.keyboard.press("ArrowDown"); + } + await keyboard.keyboard.press("Enter"); + await expect.poll(() => popupOpens()).toBe(2); + await expect.poll(() => keyboardUi.popoverOpen()).toBe(false); + + expect((await readLog(harness)).some((entry) => entry.action === "get-credential")).toBe(false); +}); + +test("where the popup cannot open, the more-logins footer names the Extensions menu and the shortcut", async ({}, testInfo) => { + harness = await startHarness(testInfo); + await stubShortcut("Ctrl+Shift+L"); + await stubOpenPopup(true); + await setMode(harness, "no-fields"); + const page = await openFixture(harness, "login.html"); + const ui = inlineOf(page); + const username = page.locator("#username"); + await openOn(page, ui, username); + await clickInline(page, ui, OPTION, 0); + + const text = `1 more login on ${FAKE_DOMAIN}: open Subclave from the browser's Extensions menu`; + const fallback = `${text}, or press Ctrl+Shift+L`; + await expect.poll(() => ui.pickerText()).toContain(fallback); + expect(await ui.count(OPTION)).toBe(0); + expect(await popupOpens()).toBe(1); + + // The page remembers: the next picker is text at once and asks no more. + await page.keyboard.press("Escape"); + await expect.poll(() => ui.popoverOpen()).toBe(false); + await username.click(); + await expect.poll(() => ui.pickerText()).toContain(fallback); + expect(await ui.count(OPTION)).toBe(0); + expect(await popupOpens()).toBe(1); + + // A browser with no openPopup at all reads the same; with the shortcut + // unbound, the text names the Extensions menu only. + await harness.worker.evaluate(() => { + const action: { openPopup?: () => Promise } = chrome.action; + delete action.openPopup; + }); + await stubShortcut(""); + const fresh = await openFixture(harness, "login.html"); + const freshUi = inlineOf(fresh); + await openOn(fresh, freshUi, fresh.locator("#username")); + await clickInline(fresh, freshUi, OPTION, 0); + await expect.poll(() => freshUi.pickerText()).toContain(text); + expect(await freshUi.pickerText()).not.toContain(", or press"); }); test("a pick fills the field's login through via inline, by pointer and by keyboard", async ({}, testInfo) => { diff --git a/extension/test/playwright.config.ts b/extension/test/playwright.config.ts index 6d48ae4..adfb933 100644 --- a/extension/test/playwright.config.ts +++ b/extension/test/playwright.config.ts @@ -25,7 +25,9 @@ import { FIXTURE_PORT } from "./support/harness"; // the API first, which is the same condition as Firefox and Chrome before // 127, and the multi-match branch then takes its documented // newest-`lastUsedAt` fallback. The real openPopup path needs a headed -// browser, so these headless specs do not exercise it. +// browser, so these headless specs do not exercise it. `inline.spec.ts` +// replaces it with a stub that counts calls (or rejects) to drive the +// picker's more-logins row. // 2. A key press synthesized over CDP does not reach Chrome's own // `chrome.commands` handling, so `fill.spec.ts` falls back to sending the // service worker's internal `{ type: "fill-command" }` message from the From 81a1276445b0bdf1cedf71b10f740fe12e488ff5 Mon Sep 17 00:00:00 2001 From: Rendy Wijaya <65700377+rendyuwu@users.noreply.github.com> Date: Tue, 6 Oct 2026 11:27:13 +0700 Subject: [PATCH 2/3] fix(vault): let the join form follow the window and accept a blank Region (#23) The join screen was a fixed 384px column that scrolled on its own, so the wheel worked only over that column. The eye button's addon in PasswordField hung 3px past its input group, and the scrolling form turned that into a horizontal scrollbar. The screen is now the scroll container and the form is m-auto up to max-w-3xl, the Settings content width. The addon overrides pr-3 has-[>button]:-mr-1 with pr-2 has-[>button]:mr-0, so the button stays where it was and nothing hangs past the group. Join vault no longer requires an S3 Region. provider_config passes the region through as typed, Settings > Sync saves it blank, and a server that takes an empty one works with it (Cloudflare R2 reads it as auto). Bucket stays required for S3: a blank one builds {endpoint}//{key} paths. connectionFieldsReady becomes missingConnectionFields, which returns the labels of the blank fields, and useMasterPassword also returns problems, one sentence per rule the password still fails. useJoinSync builds blockers from both, the credential pair and the passphrase, and derives canSubmit from it. The screen lists the blockers under the button in muted text. scripts/sync-verify.ts checks the new connection gate, including the blank Region. Closes #22 --- CHANGELOG.md | 5 +++ scripts/sync-verify.ts | 31 ++++++++----- src-tauri/src/modules/sync/engine/types.rs | 7 +-- src/modules/sync/types.ts | 30 +++++++------ src/modules/vault/JoinSyncScreen.tsx | 36 ++++++++++----- src/modules/vault/MasterPasswordFields.tsx | 18 +++++--- src/modules/vault/PasswordField.tsx | 5 ++- src/modules/vault/join/useJoinSync.ts | 51 ++++++++++++++-------- 8 files changed, 121 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 655b660..28b9f30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ The latest release only. Every earlier version: - The inline picker's "more logins" footer is now a row that opens the Subclave popup, which lists the logins saved for other hosts of the domain. Where the browser cannot open the popup from the page, the footer names the Extensions menu and the fill shortcut instead. +### Fixed + +- The "Join a synced vault" screen scrolls as a whole and its form widens with the window, up to the Settings content width. No horizontal scrollbar appears. +- Join vault no longer stays disabled on a blank S3 Region, which Settings > Sync already saves. While the button is disabled, a list under it names every field that is still empty or invalid. + ## [0.1.1] - 06-10-2026 No change to the desktop app or to the extension's code. diff --git a/scripts/sync-verify.ts b/scripts/sync-verify.ts index 25a417f..97b79f5 100644 --- a/scripts/sync-verify.ts +++ b/scripts/sync-verify.ts @@ -31,9 +31,9 @@ import type { SyncStatus, } from "@/modules/sync/types"; import { - connectionFieldsReady, DEFAULT_SYNC_CONFIG, EMPTY_SYNC_STATUS, + missingConnectionFields, } from "@/modules/sync/types"; const ROOT = join(dirname(fileURLToPath(import.meta.url)), ".."); @@ -486,9 +486,9 @@ check( !syncStore.includes("readDirty"), ); -// The join form's gate. Its whole value is that it is a checked predicate -// rather than a condition inside a component: a flipped one makes a provider -// permanently unsubmittable while the form still looks complete. +// The join form's connection gate. Its whole value is that it is a checked +// function rather than a condition inside a component: a field wrongly listed +// makes the join form refuse a connection Settings > Sync saves. { const base: SyncConfig = { enabled: true, @@ -500,19 +500,26 @@ check( cas: true, }; const webdav: SyncConfig = { ...base, provider: "webdav", region: "", bucket: "" }; - check("a webdav connection needs no region and no bucket", connectionFieldsReady(webdav, true)); + const missing = (config: SyncConfig): string => missingConnectionFields(config).join(", "); check( - "a webdav connection still needs its own credential pair", - !connectionFieldsReady(webdav, false), + "a webdav connection needs no region and no bucket", + missing(webdav) === "", + missing(webdav), ); - check("an s3 connection needs a region and a bucket", connectionFieldsReady(base, true)); + check("a complete s3 connection is ready", missing(base) === "", missing(base)); + const blankRegion = missing({ ...base, region: "" }); check( - "an s3 connection without a bucket is refused", - !connectionFieldsReady({ ...base, bucket: "" }, true), + "an s3 connection with a blank region is ready, as Settings > Sync saves one", + blankRegion === "", + blankRegion, ); + const blankBucket = missing({ ...base, bucket: " " }); + check("an s3 connection without a bucket names Bucket", blankBucket === "Bucket", blankBucket); + const blankEndpoint = missing({ ...webdav, endpoint: " " }); check( - "a connection with no endpoint is refused", - !connectionFieldsReady({ ...webdav, endpoint: " " }, true), + "a connection with no endpoint names Endpoint", + blankEndpoint === "Endpoint", + blankEndpoint, ); } diff --git a/src-tauri/src/modules/sync/engine/types.rs b/src-tauri/src/modules/sync/engine/types.rs index a28649e..fc5076f 100644 --- a/src-tauri/src/modules/sync/engine/types.rs +++ b/src-tauri/src/modules/sync/engine/types.rs @@ -135,9 +135,10 @@ pub struct SyncConfigArg { pub provider: String, pub endpoint: String, /// S3's alone. `#[serde(default)]` so a WebDAV caller does not have to send - /// an S3 field `provider_config` ignores. The S3 form still requires it - /// (`connectionFieldsReady` in `src/modules/sync/types.ts`) and - /// `provider_config` still puts it into the JSON `S3Config` deserializes. + /// an S3 field `provider_config` ignores. Neither form requires it: a server + /// that takes an empty region (Cloudflare R2 reads one as `auto`) works with + /// it blank, and `provider_config` puts it as typed into the JSON `S3Config` + /// deserializes. #[serde(default)] pub region: String, /// S3's alone, by the same default as the `region` field. diff --git a/src/modules/sync/types.ts b/src/modules/sync/types.ts index 34affb8..df93104 100644 --- a/src/modules/sync/types.ts +++ b/src/modules/sync/types.ts @@ -131,22 +131,26 @@ export function configSubset(config: SyncConfig): SyncConfigArg { } /** - * Whether the fields a provider actually needs are filled in. + * The connection fields a provider needs that are still blank, by the label the + * join form shows; empty when the connection is complete. * - * SHARED WITH `scripts/sync-verify.ts`, and that is the point: this is the one - * boolean in the join form that can make a whole provider unusable while the - * form still looks complete, so it lives where a check can drive it rather than - * inside a component. + * SHARED WITH `scripts/sync-verify.ts`, and that is the point: a field wrongly + * listed here makes a whole provider unusable from the join form while Settings + * > Sync, which has no such check, saves the same values. * - * REGION AND BUCKET ARE S3'S ALONE. A WebDAV configuration refuses unknown - * fields, so demanding them for a WebDAV server would make the form impossible - * to submit. + * REGION IS NEVER LISTED. `provider_config` in + * `src-tauri/src/modules/sync/engine/commands.rs` passes it through as typed, a + * server that takes an empty one (Cloudflare R2 reads it as `auto`) works with + * it blank, and Settings > Sync saves it blank. + * + * BUCKET IS S3'S ALONE. A WebDAV configuration refuses unknown fields, so + * demanding one for a WebDAV server would make the form impossible to submit. */ -export function connectionFieldsReady(config: SyncConfig, credentialsReady: boolean): boolean { - if (config.endpoint.trim().length === 0) return false; - if (!credentialsReady) return false; - if (config.provider === "webdav") return true; - return config.region.trim().length > 0 && config.bucket.trim().length > 0; +export function missingConnectionFields(config: SyncConfig): string[] { + const missing: string[] = []; + if (config.endpoint.trim().length === 0) missing.push("Endpoint"); + if (config.provider !== "webdav" && config.bucket.trim().length === 0) missing.push("Bucket"); + return missing; } /** A device with sync never configured. Off, and naming nothing. */ diff --git a/src/modules/vault/JoinSyncScreen.tsx b/src/modules/vault/JoinSyncScreen.tsx index 6ec178e..2bd1dd8 100644 --- a/src/modules/vault/JoinSyncScreen.tsx +++ b/src/modules/vault/JoinSyncScreen.tsx @@ -24,20 +24,21 @@ export function JoinSyncScreen({ onCreateInstead }: { onCreateInstead: () => voi const [passphrase, setPassphrase] = useState(""); const masterPassword = useMasterPassword(); - const { submit, busy, error, fresh, canSubmit } = useJoinSync({ + const { submit, busy, error, fresh, canSubmit, blockers } = useJoinSync({ config, credentials, passphrase, password: masterPassword.password, - masterPasswordReady: masterPassword.ready, + masterPasswordProblems: masterPassword.problems, }); + // The whole screen scrolls, not the form, so the wheel works anywhere on it. + // `m-auto` centres a short form and collapses to 0 on a tall one, which keeps + // its top reachable. `max-w-3xl` is the Settings window's content width, so + // the rows shared with Settings > Sync wrap the same in both. return ( -
-
+
+

Join a synced vault

@@ -85,10 +86,23 @@ export function JoinSyncScreen({ onCreateInstead }: { onCreateInstead: () => voi

) : null} - +
+ + {blockers.length > 0 ? ( +
    + {blockers.map((blocker) => ( +
  • {blocker}
    • + ))} +
+ ) : null} +
); diff --git a/src/modules/vault/MasterPasswordFields.tsx b/src/modules/vault/MasterPasswordFields.tsx index bba2bed..1461937 100644 --- a/src/modules/vault/MasterPasswordFields.tsx +++ b/src/modules/vault/MasterPasswordFields.tsx @@ -10,8 +10,8 @@ export const MIN_PASSWORD_LENGTH = 8; /** * The master-password half of a first-run screen: the password, its - * confirmation, and the no-recovery acknowledgement, plus the one boolean both - * screens gate Submit on. + * confirmation, and the no-recovery acknowledgement, plus `problems`, every + * rule it still fails, and `ready`, which is `problems` being empty. */ export type MasterPasswordDraft = { password: string; @@ -20,7 +20,9 @@ export type MasterPasswordDraft = { setPassword: (value: string) => void; setConfirm: (value: string) => void; setAcknowledged: (value: boolean) => void; - /** Long enough, confirmed, and acknowledged. A screen still has to add `!busy`. */ + /** Every rule the password still fails, one sentence each, in the order the fields render; empty once `ready`. */ + problems: string[]; + /** Long enough, confirmed, and acknowledged: `problems` is empty. A screen still has to add `!busy`. */ ready: boolean; }; @@ -33,9 +35,14 @@ export function useMasterPassword(): MasterPasswordDraft { const [confirm, setConfirm] = useState(""); const [acknowledged, setAcknowledged] = useState(false); + const problems: string[] = []; // Counted by code point rather than UTF-16 unit, so four emoji are four // characters and not eight. - const longEnough = [...password].length >= MIN_PASSWORD_LENGTH; + if ([...password].length < MIN_PASSWORD_LENGTH) { + problems.push(`Master password needs at least ${MIN_PASSWORD_LENGTH} characters.`); + } + if (confirm !== password) problems.push("The confirmation doesn't match the master password."); + if (!acknowledged) problems.push("The no-recovery box isn't ticked."); return { password, @@ -44,7 +51,8 @@ export function useMasterPassword(): MasterPasswordDraft { setPassword, setConfirm, setAcknowledged, - ready: longEnough && password === confirm && acknowledged, + problems, + ready: problems.length === 0, }; } diff --git a/src/modules/vault/PasswordField.tsx b/src/modules/vault/PasswordField.tsx index 782b22b..ec9a2f4 100644 --- a/src/modules/vault/PasswordField.tsx +++ b/src/modules/vault/PasswordField.tsx @@ -50,7 +50,10 @@ export function PasswordField({ onKeyDown={onKeyDown} onKeyUp={onKeyUp} /> - + {/* Overrides the addon's `pr-3 has-[>button]:-mr-1`: the button sits in the + same place, but the addon no longer hangs 3px past the group, which a + scrolling parent turns into a horizontal scrollbar. */} +