fix: improve hostname parsing logic in getSiteName function#1417
Open
Hank076 wants to merge 175 commits into
Open
fix: improve hostname parsing logic in getSiteName function#1417Hank076 wants to merge 175 commits into
Hank076 wants to merge 175 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
--generate-notes builds notes from merged PRs, but this fork commits directly to dev, so generated notes would be nearly empty. The tag annotation is author-controlled and always has content.
actions/checkout flattens the annotated tag to its commit (actions/checkout#290), so --notes-from-tag fell back to the commit message. Force-fetching the real tag ref restores the annotation.
GitHub deprecated Node 20 on runners in 2025-09; the v4 actions emitted deprecation annotations on every run.
qrcode-reader has been unmaintained since 2017 and was only kept as a first-attempt decoder with jsqr as its fallback. Decode QR codes with jsqr directly in both the screen-capture (content.ts) and file-import (QrImport.vue) paths, and drop the now-unused dependency.
Upgrade eslint 8.57.0 -> 10.7.0 and @typescript-eslint 7.15.0 -> 8.63.0 (both EOL/outdated) and replace .eslintrc.js + .eslintignore with a flat eslint.config.mjs. Rule behavior is kept equivalent to the v7/v8 baseline (new v10/v8 recommended rules that would change existing behavior are explicitly disabled); ignores are rewritten as **-prefixed globs so nested build output (test/chrome/js, test/firefox/js, etc.) stays excluded the way the old gitignore-style .eslintignore did. scripts/build.sh drops the now-unsupported `--ext .js,.ts` CLI flag; file matching is handled by eslint.config.mjs's `files` globs instead. src/definitions/module-interface.d.ts: the disable comment referenced `@typescript-eslint/ban-types`, which typescript-eslint v8 split into several rules; repointed to `@typescript-eslint/no-unsafe-function-type` to match the actual rule now firing on the `Function` type usages below.
Same-major, low-risk dependency refresh: webpack ^5.94.0->^5.108.4, sass ^1.26.11->^1.101.0, ts-loader ^9.0.0->^9.6.2, @types/chrome ^0.0.266->^0.2.2, lodash ^4.17.21->^4.18.1. @types/chrome 0.2.2 tightens chrome.storage.*.get()'s default generic from any to unknown and renames chrome.windows.createTypeEnum to CreateType; fixed with minimal `as string`/typed-callback casts in src/background.ts, src/import.ts, src/store/Accounts.ts and src/models/storage.ts (no logic changes). @types/chrome 0.2.2 also uses NoInfer, a TypeScript 5.4+ builtin not present in the pinned typescript@^5.0.0 (5.0.3), which broke the standalone `tsc scripts/test-runner.ts` compile in scripts/build.sh (the main webpack build path is unaffected since ts-loader runs transpileOnly with fork-ts-checker, which skips declaration/global diagnostics by default). Added --skipLibCheck to that one tsc invocation as a scoped workaround -- it only affects third-party .d.ts checking for this tooling script, not project source. Remove once typescript is bumped to >=5.4 in a future toolchain upgrade.
TS 5.9 tightens option validation: `--moduleResolution nodenext` now requires `--module nodenext` (TS5110), so build.sh's test-runner tsc invocation is updated accordingly. package.json has no "type": "module", so output for .ts files under --module nodenext remains CommonJS — node scripts/test-runner.js (CJS require semantics) is unaffected; npm test exercises the compiled output directly and passes. --skipLibCheck stays: dropping it surfaces a pre-existing @types/sinon-chrome incompatibility (Event<Function> does not satisfy the sinon-chrome Event constraint), unrelated to this TS bump.
Prepares for prettier 3 output formatting in a follow-up commit; kept separate so the reformat commit stays purely mechanical.
prettier 3 turns "no parser could be inferred" into a fatal error (prettier 2 only warned). src/test/test.ts.disabled is an upstream dead test file with no recognizable extension that was silently tolerated before; exclude it via the standard ignore mechanism instead of touching build.sh/CI globs or deleting the file.
Run prettier --write over the same scope as scripts/build.sh (STYLEFILES) after bumping to prettier 3. Output-only diff — mainly trailingComma additions (new default: "all") and re-wrapped long lines; no logic changes.
…scripts) - src/test/test.ts.disabled: 2020 upstream leftover (63ea3e5), zero references anywhere; not picked up by webpack or test-runner. - scripts/release.sh, scripts/tag.sh: Travis CI-era release/tag automation, zero references in .github/workflows or package.json; fully superseded by release.yml (2026-07 GitHub Actions migration). tag.sh was already broken (referenced a deploy-key.gpg that no longer exists in the repo). - .prettierignore: its only entry (*.disabled) existed solely to work around test.ts.disabled failing prettier 3's parser inference; now empty and removed with it.
…vendored equivalents All three were unmaintained (url-loader's repo is archived; base64-loader saw its last release years ago; vue-svg-loader never left Vue-3 beta after 2020-09). Removes them entirely rather than pinning dead dependencies. - .wasm (argon2-browser's binary): webpack 5 asset/inline with a custom base64 generator, reproducing base64-loader's exact output — a plain data URI here would break at runtime, since argon2-browser's decodeWasmBinary() calls atob() directly on the required module's value. - .png/.jpe?g/.gif: asset/inline, matching url-loader's default (no `limit` set = always inline as base64, never falls back to a separate file). - .svg: a vendored local loader (webpack-loaders/vue-svg-loader.js, MIT, based on @ifcanduela/vue-svg-loader) instead of an npm dependency, since every viable replacement candidate is itself a single-release, near-abandoned package — vendoring the ~15 lines keeps the supply chain at zero added packages. Fixed a Windows path-separator bug in the process (upstream only strips "/", not "\\", which corrupted the generated SFC on Windows). Also drops `module.noParse: /\.wasm$/`, a base64-loader-specific setting that silently broke asset/inline for .wasm (verified via isolated repro: it caused webpack to emit a separate asset/resource file + URL instead of running the inline generator).
…ck-merge majors
webpack-cli 5.0.1 -> 7.2.1, fork-ts-checker-webpack-plugin 6.5.3 -> 9.1.0,
webpack-merge 5.8.0 -> 6.0.1. All three are devDependencies used only by
scripts/build.sh and webpack.*.js.
No config migration needed: webpack.config.js / webpack.dev.js /
webpack.prod.js all load unchanged under webpack-cli 7's dual ESM/CJS
config loader, fork-ts-checker 9's default options still cover this
project's `new ForkTsCheckerWebpackPlugin()` usage, and webpack-merge 6
keeps a CommonJS `require('webpack-merge').merge` export compatible with
existing usage.
Verified: eslint clean, chrome + firefox builds pass, npm test 29/29
passing. Confirmed fork-ts-checker 9 still fails the build on a
deliberately injected type error (reverted before commit).
webpack.watch.js remains broken (missing webpack-extension-reloader
dependency, pre-existing per its own TODO comment) - unrelated to this
upgrade; webpack-cli 7 loads and fails on it identically to before.
mocha 10.2.0 -> 11.7.6 and sinon 17.0.1 -> 22.0.0 (with matching @types/mocha and @types/sinon), keeping sinon-chai on 3.7.0 since 4.x requires chai ^5/^6 and this project pins chai to 4.x. mocha 11's browser bundle now requires Node core modules via the "node:" URI scheme (node:fs, node:path, node:util, node:events), which webpack treats as an unhandled resource scheme rather than a normal module request, bypassing resolve.fallback entirely. Strip the prefix via NormalModuleReplacementPlugin in webpack.dev.js so it falls back to the existing bare-name polyfills, and add an "events" polyfill fallback (none of the test/sinon API surface changed across the sinon majors).
Only consumer was the Travis-era release.sh (removed in de93d88); the fork's PKCE architecture ships zero secrets and release.yml never decrypts anything. The blob was upstream's, undecryptable here.
Dart Sass @import is deprecated (removed in 3.0.0), and every build emitted 8 deprecation warnings from popup/options/permissions/import entries pulling in _ui.scss -> _tokens.scss. Replaced with @use via `sass-migrator module --migrate-deps`, no manual namespacing needed since no Sass-level members (vars/mixins/functions) cross module boundaries; only CSS custom properties and @extend are shared. Verified: build emits zero sass deprecation warnings; compiled CSS is byte-identical except one inert #info .button-small/#security-* extend leak that never matched any real DOM node and was already overridden by a later explicit rule; npm test still 29/29 passing.
webpack-extension-reloader was never installed and does not support webpack 5, so `npm run dev:chrome` failed with Cannot find module. Rely on webpack's built-in watch (already enabled) instead, and update the webpack-merge import to the v6 named-export form.
jsqr is unmaintained (last release 1.4.0, 2021-04) and ships a wrong version-23 alignment pattern table (74 where ISO/IEC 18004 says 78), confirmed by cross-checking thonky, zxing sources, and the Nayuki formula. Replace it with the actively maintained @zxing/library behind a shared leaf module (src/qr-decoder.ts, no src/models imports) used by both the import page and the injected content script, plus an encode->decode round-trip test. Known trade-off: content.js grows 311 KiB -> ~1.9 MiB (injected on demand only, not statically). Planned slimming tracked in AGENTS.md: move decoding into the background service worker, which already holds the capture dataURL.
Decode captured QR regions in the background worker instead of the injected content script: content.ts now sends the selection box plus window.innerWidth with getCapture, and background crops the captured bitmap via OffscreenCanvas and decodes it directly, feeding the existing getTotp flow. This drops @zxing/library from content.js (1.87 MiB back to ~54 KiB) so autofill injection no longer carries an unused decoder, and saves one message round-trip (sendCaptureUrl is gone). The crop/DPR math lives in computeQrCropRegion (qr-decoder.ts leaf module) with unit tests in utils.test.ts. Decoding failures now message the tab with action "errorqr"; content.ts finally handles that action, which also un-silences the pre-existing errorqr senders in getTotp. The data-URL-to-Blob conversion uses atob rather than fetch() because the extension CSP (connect-src) blocks data: fetches inside the MV3 service worker.
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.
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