fix: improve hostname parsing logic in getSiteName function - #1417
Open
Hank076 wants to merge 197 commits into
Open
fix: improve hostname parsing logic in getSiteName function#1417Hank076 wants to merge 197 commits into
Hank076 wants to merge 197 commits into
Conversation
EntryStorage.import() ran parseInt() on the type and algorithm fields,
but both are persisted as their enum *names* ("hotp", "SHA256"), so
parseInt always returned NaN and fell back to the defaults. Every
imported entry therefore became TOTP/SHA1, losing HOTP/Steam/Battle/hex
types and SHA256/SHA512 digests.
Map the stored name back to the enum (case-insensitive for algorithm),
guarding with a typeof-number check so legacy numeric data still falls
back safely.
Refs Authenticator-Extension#1292, Authenticator-Extension#405, Authenticator-Extension#1442, Authenticator-Extension#1294, Authenticator-Extension#1089
second % 60 keeps the sign of the dividend in JS, so a negative clock offset could leave state.second negative; the old "+ 60" only covered offsets down to -60. Use a positive modulo so any offset stays in 0..59. Refs Authenticator-Extension#1310
The otpauth importer rejected periods that were > 60 or not a divisor of 60, silently dropping valid values like 45, 90 or 120 and falling back to 30s, which produces wrong codes. Validate as a positive integer instead. Refs Authenticator-Extension#1271, Authenticator-Extension#1508
Toggling Smart Filter off also fired the "smart filter loosely matches the domain name" notification. Show it only when the toggle is turned on. Refs Authenticator-Extension#1282
An unset autolock falls back to a 30-minute default in setAutolock(), but the advisor treated "unset" the same as "disabled" and kept showing the warning. Only warn when autolock is explicitly set to 0. Refs Authenticator-Extension#1281
The algorithm and digits <select>s used v-model without the .number modifier, so the in-memory entry held string values (e.g. "2"). The generator compares the algorithm with === against the numeric enum, so every non-SHA1 choice fell through to SHA1 until the entry was reloaded from storage (which normalises the type). Add .number like the type select already does. Refs Authenticator-Extension#1184
The OneDrive OAuth code-for-token POST had no body at all, so Microsoft's token endpoint always rejected it and sign-in never completed. Unlike Google's endpoint (which accepts the params in the query string), Microsoft requires them in the x-www-form-urlencoded body. Add client_id, client_secret, code, redirect_uri and grant_type. Refs Authenticator-Extension#1494, Authenticator-Extension#1369, Authenticator-Extension#1408, Authenticator-Extension#1202, Authenticator-Extension#1424
The search box was only revealed by the clearFilter action, so a large account list never showed it on initial load or after unlocking with a passphrase. Reveal it after entries load when there are >= 10 of them and a smart filter isn't currently applied. Refs Authenticator-Extension#1496, Authenticator-Extension#1400
andOTP exports a flat JSON array using a "label" field instead of the keyed object with "account" that the importer expects, so its backups imported as empty/garbled. Detect the array form and map each entry to RawOTPStorage (label -> account with the redundant issuer prefix stripped, type lower-cased; EntryStorage.import normalises type/algorithm names). Refs Authenticator-Extension#1304
getQrUrl encoded the whole label, turning the issuer/account separator into %3A. Several authenticators (including Google Authenticator) split the label on a literal ":" and reject the %3A form, so the exported QR scanned as invalid. Encode issuer and account separately and strip the internal "::host" match suffix, matching the text backup and the otpauth spec. Refs Authenticator-Extension#1302
Autofill collected every text/number/tel/password input regardless of visibility, so the code could be written into a hidden honeypot or an off-screen field instead of the real 2FA box. Filter candidates by checkVisibility() (guarded for browsers without it). Refs Authenticator-Extension#1273, Authenticator-Extension#1136
moveCode received from/to positions in the displayed (pinned-first) order but spliced and re-indexed the raw entries array, so dragging scrambled the list whenever any entry was pinned. Reorder the displayed view first, then write back sequential indices. No change when nothing is pinned. Refs Authenticator-Extension#1312, Authenticator-Extension#1428
The issuer/account edit inputs let keydown events bubble to the list's arrow-key navigation handlers, so pressing left/right to move the text cursor moved focus to another entry, blurred the field and committed the edit (e.g. saving an emptied account). Stop keydown propagation on those inputs. Refs Authenticator-Extension#495
…ding
failedCount filtered Promise.allSettled results with `!res`, but those are
always-truthy {status,value} objects, so the count was always 0 and both
failure branches were dead code — a fully failed otpauth-migration import
still reported "migrationsuccess". Inspect status/value instead.
Refs code review §2.2
chrome.storage.sync.set was awaited with no error handling, so hitting the sync quota (MAX_ITEMS / QUOTA_BYTES_PER_ITEM) rejected as a silent unhandled rejection while the entry stayed in the Vuex view but was never persisted — the long-standing "can't add more than N accounts" data loss. Wrap the write with a clear error, and surface it when adding an account so a save that failed no longer shows a phantom entry. Refs code review §2.1
… codes isMatchedEntry matched the bound host with an unanchored indexOf and matched the page-controlled <title> as a substring, so a hostile page at google.com.attacker.com (or just <title>Google</title>) could be the sole match and receive the user's live OTP via the autofill command. Add a strict mode (used by autofill) that anchors the host match to a real domain boundary and drops the title hint; display filtering stays loose. Refs code review §2.3
nextCode() guarded on this.$store.state.style.hotpDisabled, but the flag lives one level deeper (state.style.style.hotpDisabled), so the guard was always undefined and every click advanced and persisted the HOTP counter with no rate limit. The disabled CSS class also read a misspelled `hotpDiabled` and never applied. Fix both. Refs code review §3.4
…ckup getOneLineOtpBackupFile reassigned otpStorage.issuer/account in place, but that object is the live Vuex export state, so generating a one-line backup corrupted the stored issuer/account (colons stripped, values %-encoded) for any subsequent JSON backup and double-encoded on a repeat download. Compute sanitized values into locals instead. Refs code review §3.3
…tion The hand-rolled protobuf walker trusted every attacker-controlled length byte and read past the buffer (subBytesArray returns undefined), and indexed the algorithm/digits/type lookup tables with raw bytes — out-of-range values produced otpauth://undefined/...&algorithm=undefined, silently importing entries that generate wrong codes. decodeURIComponent on a missing/garbled data= param could also throw and abort the whole import batch. Guard the data= extraction and decode, bounds-check each field before reading, skip entries with out-of-range algorithm/digits/type, and wrap the caller so one bad payload can't abort a mixed import. Refs code review §3.2
The confirm action added an anonymous window "confirm" listener on every dispatch and never removed it, so listeners accumulated and a single confirm event re-fired every stale one, resolving old promises unexpectedly. Use a named handler that removes itself when it fires. Refs code review §3.5
applyPassphrase switches to LoadingPage before decrypting/migrating, which can throw (e.g. "argon2 did not return a hash!"). The caller awaited without catching, so the error became an unhandled rejection and the UI was stuck on LoadingPage with no way back. Catch it, return to EnterPasswordPage and alert. Refs code review §3.6
Each argon2 call added an anonymous window "message" listener that was never removed and resolved on the first message to arrive, with no link between request and reply — overlapping hash/verify calls (e.g. the v3 multi-key unlock loop) could resolve with the wrong response, and listeners piled up. Route every call through one helper that tags the request with a unique id, only accepts the reply from the sandbox iframe matching that id, removes its listener, and times out instead of hanging forever. Collapse the six copied inline postMessage blocks (Accounts, import) onto argonHash/argonVerify. Refs code review §3.1, §3.6 (sandbox timeout)
crypto-js is imported and run at runtime (OTP secret AES encrypt/decrypt in key-utilities, encryption, migration, etc.) but was declared under devDependencies, so an SBOM or `npm install --omit=dev` would drop the actual crypto library. @types/lodash is type-only and was in dependencies. Swap them to the correct sections. Refs code review §5, §6
The base webpack config sets devtool: "source-map" for development; the prod config merged it through, so release artifacts shipped full source maps (and build.sh copies the .map files into the package). Override devtool: false for production. Refs code review §5
The fork notice ended with `read -n1` (Press any key to continue), which hangs any non-interactive/automated build on a fork remote. Keep the notice but drop the blocking read. Refs code review §5
…mport decryptBackupData JSON.parsed the AES-decrypted v2/v3 payload with no guard, so one entry that decrypts to invalid UTF-8/JSON threw and aborted the whole backup import. Wrap it and continue past the bad entry. Refs code review §6
new Date(header).getTime() returns NaN for an unparseable Date header, which made the offset NaN and fell through to "clock_too_far_off" — a misleading state. Return "updateFailure" instead. Refs code review §6
The Dropbox upload handler only special-cased 401 and otherwise JSON-parsed the body, so a 5xx or HTML error page would throw or be misread. Reject non-2xx responses with a clear error. Refs code review §6
The post-drag getCapture sendMessage was fire-and-forget; when the MV3 service worker fails to start (or the receiving end is otherwise gone), the rejection was swallowed and the scan looked like nothing happened. Alert with the existing capture_failed message so the user gets feedback. Normal decode failures are unaffected: the background listener resolves the promise with undefined; it only rejects when no receiver exists.
Scanning a QR that decodes to something other than an otpauth:// URI used to alert the raw decoded text (e.g. a URL), which reads like a glitch. Send the new error_not_otpauth i18n message through the existing "text" alert channel instead; the content script is untouched. The key is added to all 44 locales (41 translated, fy/kaa fall back to English).
docs/ holds untracked local notes and one-off debug scripts; 'eslint .' during builds failed on them with 125 no-undef errors.
Firefox manifests are bumped in the AMO compliance commit that follows.
- add mandatory browser_specific_settings.gecko.data_collection_permissions (required: none, optional: authenticationInfo), enforced for new extensions since 2025-11 - prune stale Google Drive / OneDrive host permissions and CSP connect-src entries, matching the chrome manifest cleanup in b9614f4 - replace upstream gecko.id with our own GUID {aa04367e-4dc8-4833-87a1-43ee93ce9a42}; the upstream id belongs to the original AMO listing and must not be reused - bump firefox manifests to 8.0.4
Clears the only first-party UNSAFE_VAR_ASSIGNMENT warning in the AMO validation report; rendered output is unchanged.
Build firefox first and zip immediately: scripts/build.sh wipes the other platform's output directory on every run.
prettier 3 ships its bin as bin/prettier.cjs; the hardcoded bin-prettier.js path from the 2.x era exits 127 since the 3.x bump.
Low-risk minor bumps verified safe against 2026-07 upstream state: - vue + @vue/compiler-sfc 3.4.21 -> 3.5.40 (official minor, no breaking changes) - @vue/test-utils 2.4.5 -> 2.4.11 - @typescript-eslint/* 8.63 -> 8.64 (lint clean) - qrcode-generator 1.4.4 -> 1.5.2 (zero code diff between tags) npm test green, eslint clean.
Last v24 release (2026-05-11). v25 is ESM-only and incompatible with the CJS-compiled test-runner, so v24 is the ceiling until the runner moves to ESM. Verified: --load-extension still works on Chrome for Testing (the Chrome 137+ removal only hits branded Chrome), and the runner uses no v23+ changed APIs. 37 tests green.
vuedraggable 4.1.0 has had no release since 2021 and its npm latest tag still points at the Vue 2 build. vue-draggable-plus (0.6.1) is the actively maintained Sortable.js wrapper for Vue 3 with the same option/event surface. MainBody.vue keeps identical bindings (v-model, handle, disabled, keydown handlers); the #item slot becomes a plain v-for with the same :key. 48 tests green, chrome build OK, independently verified prop/event parity.
Vuex is in maintenance mode; Pinia is the official Vue default. Wave 1 mounts Pinia 4 alongside Vuex and converts the three leaf modules (no cross-module dependents) to setup stores. Cross-module calls from the still-Vuex Accounts/Notification modules now invoke the Pinia stores directly. Build-chain changes required by Pinia 4 being ESM-only: - tsconfig moduleResolution node -> bundler - temporary paths shim for vuex (its exports map lacks a types condition under bundler resolution; remove with vuex in wave 3) - @pinia/testing 2.0.1 with sinon spies for component tests 37 tests green, eslint clean, independently verified (6/6).
…ions to Pinia (wave 2/3) Converts the five independent modules to setup stores. Async-loaded modules (Advisor/Backup/Menu, formerly awaited via getModule) keep their mount-before-ready semantics: each store exposes an async init() that popup.ts awaits before app.mount(), in the original module order. The standalone permissions entry now mounts Pinia and no longer imports vuex. Vuex remains only for accounts (wave 3). 37 tests green, eslint clean, independently verified (6/6).
Converts the largest module (17 state refs, 4 getters, 20 mutations
folded into actions, 7 actions + init) to a setup store. The
encryption Map keeps its .get()/.set() API throughout (Known Quirk
2), and the sorted-entries getter is renamed sortedEntries to avoid
colliding with the entries state in Pinia's flat namespace.
vuex is uninstalled: the tsconfig paths shim added in wave 1 is
gone, and vue.d.ts drops the $store augmentation (with export {} to
stay a module - removing the vuex import otherwise turns the file
into a script whose runtime-core declaration shadows the real one).
BREAKING CHANGE: state management is now Pinia; $store is no longer
available on component instances.
37 tests green, eslint clean, chrome build OK, adversarially
verified 7/7 including the addCode/applyPassphrase/changePassphrase
encryption paths.
Module, VuexConstructor, MenuState, AccountsState, NotificationState, BackupState, AdvisorState and PermissionsState had zero references after the Vuex->Pinia migration. StyleState is kept, it is still used by src/store/Style.ts.
The .entries gap comment still named the old vuedraggable package, which was replaced by vue-draggable-plus (VueDraggable component) during the Pinia migration.
No sender in src ever posts a "getTotp" runtime message; the branch was dead. getTotp() itself is kept, it is still called internally by decodeCapture() and recursively for otpauth-migration URLs.
legacy localStorage-era settings may hold advisorIgnoreList as a JSON string; init() previously assigned it raw, so dismissInsight()'s .push() threw TypeError for those users and could persist the string back to storage.
Phase 0 of the crypto-js removal plan (docs/plans/ crypto-js-migration-plan.md): freeze the current behaviour before touching any implementation. - 6 legacy AES ciphertexts generated with the in-tree crypto-js (ascii/chinese/emoji/73-byte passphrases, unicode and empty plaintexts) asserted through decryptString - wrong-passphrase baseline: CryptoJS returns "" -> decryptString yields null (locked as the semantic the EVP replacement must keep, same for the empty-plaintext || null quirk) - RFC 4226 (10 counters) and RFC 6238 (sha1/256/512 x 6 timestamps) official vectors via the hhex counter path, cross-checked against an independent node:crypto reimplementation 74 tests green.
…(phase 1) migration.ts decodes otpauth-migration base64 with atob directly; the GOST branch feeds GostEngine via @noble/hashes hexToBytes (GostEngine takes plain byte arrays - crypto-js WordArray was only plumbing). Adds @noble/hashes 2.2.0 (Cure53-audited, zero-dep), shared by the later HMAC and EVP phases. HMAC paths untouched. 74 tests green incl. GOST suite.
HmacSHA1/256/512 + enc.Hex become hmac(sha1|sha256|sha512) with hexToBytes/bytesToHex. The call chain stays fully synchronous (otp.ts forbids async here). Hex inputs are already even-length (key padding, time padStart) so noble's stricter parser is safe. Equivalence proven by the phase-0 baseline: RFC 4226 10/10 and RFC 6238 18/18 vectors pass unchanged; 74 tests green.
… crypto-js (phases 3-4) New leaf module legacy-decrypt.ts reimplements the crypto-js OpenSSL format: Salted__ framing, EVP_BytesToKey (MD5 via @noble/hashes, 1 iteration, 32-byte key + 16-byte IV) and WebCrypto AES-CBC decrypt. Two variants mirror the two historical outputs: legacyDecrypt (Utf8, used by decryptString) and legacyDecryptToHex (hex - CryptoJS WordArray.toString() defaults to Hex, so the v2 key sites in Accounts/TextImport/FileImport must stay byte-identical). Wrong-passphrase behaviour converges to null/empty (CBC padding reject) instead of occasional garbage; every caller already treats that as failure. The AES-GCM v4 write path is untouched. BREAKING CHANGE: crypto-js and @types/crypto-js are removed. Verified: 6 cross-decryption fixtures + RFC 4226/6238 vectors from the phase-0 baseline, 74 tests green, chrome build OK, adversarial review 7/7 against crypto-js sources.
npm ci + npm update for vue/@vue/compiler-sfc 3.5.40, eslint 10.8.0, prettier 3.9.6, sass 1.102.0, sinon 22.1.0, webpack 5.109.0. Verified with a green chrome build (webpack compiled successfully).
Both locales had the English source text as a placeholder. Wording aligned with existing keys in each file (fy: errorqr/errorsecret; kaa: errorqr/add_qr). kaa "jaramli"/"skanerlengen" derived by morphology, pending native-speaker review.
qrcode-generator 2.0.4 CJS entry and .d.ts are byte-identical to 1.5.2 (v2 only adds an ESM dual-module exports map), so no call-site changes needed. nyc had no .nycrc and no script references. Verified: chrome build green, 74 tests passing.
npm 12+ blocks install scripts by default, so puppeteer Chromium is no longer downloaded by npm ci; document the approve+rebuild step. Also note per-target build output dirs wiping each other and the manual-reload behavior of dev:chrome.
The list claimed Advisor as a fork addition, but src/models/advisor.ts and argon2-browser both exist in upstream 9d9660b. Replace the list with differences verified against upstream sources: - Web Crypto AES-GCM replacing unauthenticated crypto-js CBC - host-bound autofill (upstream matched on the page title, which the visited site controls) - master password gating cloud upload - Dropbox PKCE with zero shipped secrets - QR decoding moved to the background worker (~1.9 MiB -> ~55 KiB injected) plus four removed OAuth host permissions - qrcode-reader/jsqr consolidated onto @zxing/library, qrcode-generator upgraded to v2
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.
Fix IP address handling priority