feat(extension): add offline authenticator for chrome and firefox - #116
Conversation
Codes are generated locally from an encrypted vault instead of being polled, so they keep working when the server does not. Secrets come from a new admin-only export route, are sealed with AES-256-GCM under a passphrase-derived key, and accounts added in the extension sync back upstream. Server-side deletions mirror into a local trash rather than dropping the secret outright, and a release workflow builds store-submittable packages whenever the extension version changes on main.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR adds a new browser extension application supporting offline-first TOTP code generation with encrypted local vault storage. It includes cryptography primitives, vault entry management, server synchronization, a background worker with alarm-driven periodic sync and auto-lock, popup and options UI pages, setup/unlock/add/edit account flows, and otpauth URI parsing. Build tooling covers Vite configuration, manifest generation for Chrome and Firefox, icon generation, packaging scripts, and a GitHub Actions release workflow. On the server side, it adds an admin-authenticated authenticator export endpoint, an 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-extension.yml:
- Around line 11-12: Update the workflow-level permissions to contents: read,
add contents: write specifically to the release job, and set
persist-credentials: false on checkout steps in prepare and build that only
require repository access. Keep the release checkout’s credential behavior
unchanged unless it does not need the write token.
- Around line 36-49: Update the workflow step containing current_version and
previous_version to pass github.event_name and github.event.before through env,
then reference those shell variables instead of embedding expressions in the run
script. Handle an all-zero or otherwise unresolvable previous SHA by treating
the previous version as the current version, so the comparison reports no
version change when no prior package version is available.
In `@apps/extension/scripts/generate-icons.ts`:
- Around line 135-141: Update the row buffer creation inside the icon-generation
loop to add pixels.byteOffset to the underlying-buffer offset before copying
each row. Preserve the existing stride and destination offsets so both full
arrays and subarray views produce correct PNG output.
In `@apps/extension/scripts/manifest.ts`:
- Line 69: Align the Firefox minimum version with the manifest’s
optional_host_permissions key: either emit the equivalent optional hosts through
optional_permissions for Firefox 115.0–127.0, or update
browser_specific_settings.gecko.strict_min_version to "128.0" if
optional_host_permissions remains the sole key.
In `@apps/extension/scripts/package.ts`:
- Around line 27-29: Update the FIXED_MTIME definition to construct a Date using
explicit local calendar fields for January 1, 1980, rather than Date.UTC.
Preserve the fixed timestamp’s reproducibility while ensuring fflate’s
local-field encoding remains within the ZIP format’s valid 1980 minimum in every
timezone.
In `@apps/extension/src/background/index.ts`:
- Around line 62-72: Update handleSetup to read the existing vault record before
calling createVault, and guard creation using the worker’s existing single-vault
invariant. If a record already exists, do not replace it or modify vault state;
only create and store a key, then sync when no record is present.
- Around line 93-122: Preserve the mutation-then-sync flow in the addAccounts,
editAccount, deleteAccount, and restoreAccount cases, but stop swallowing sync
failures with catch(() => undefined). Add or reuse the syncQuietly helper with
the same error-recording behavior used by the tick path, then replace each
direct sync catch with await syncQuietly(key) so failed pushes update
lastSyncError and sync state.
In `@apps/extension/src/components/add-account-dialog.tsx`:
- Around line 216-228: Validate and clamp digits and period in the Add button’s
submit callback before constructing the account passed to submit/onAdd. Parse
them as integers, reject non-integer or out-of-range values (digits 6–10, period
10–120), and do not invoke onAdd for invalid input; preserve the existing
defaults only where appropriate.
In `@apps/extension/src/hooks/use-codes.ts`:
- Around line 33-36: Update the useEffect refresh logic around computeAll and
setCodes to align updates with wall-clock second boundaries instead of starting
setInterval immediately. Schedule the first refresh for the next second
boundary, then continue refreshing once per second, while preserving cleanup of
all timers and the initial immediate code computation.
In `@apps/extension/src/hooks/use-vault-state.ts`:
- Around line 76-81: Update the storage change listener in the useVaultState
effect to inspect the changed area and keys, responding only to local storage
changes containing VAULT_KEY or PREFERENCES_KEY. Export those key constants from
../lib/storage and use them in the filter, leaving unrelated areas and keys
without calling refresh.
In `@apps/extension/src/lib/browser.ts`:
- Around line 21-24: Update originPattern to use url.hostname rather than
url.host when constructing the host permission pattern, so explicit API ports
are omitted while preserving the protocol and wildcard suffix.
In `@apps/extension/src/lib/clipboard.ts`:
- Around line 24-28: Update copyText’s execCommand("copy") handling to inspect
its boolean result and throw when it returns false, while preserving the finally
cleanup that removes scratch. Ensure failures reject so callers do not report a
successful copy.
In `@apps/extension/src/lib/crypto.test.ts`:
- Around line 36-42: Await the rejection assertion in the test “a wrong
passphrase fails authentication rather than returning garbage” by awaiting the
promise returned from expect(decryptJson(...)).rejects.toThrow(). Keep the
existing wrong-key setup and expected rejection unchanged.
In `@apps/extension/src/lib/entries.ts`:
- Around line 104-116: Update the trash-entry reconstruction in the visible
entries flow so serverId and serverUpdatedAt are cleared whenever pendingDelete
is false, including local deletions; retain the existing server identity only
while deletion remains pending. Add a regression test covering reason "local"
with pendingDelete false and verify both server fields are null.
In `@apps/extension/src/lib/sync.test.ts`:
- Around line 141-144: Await both rejection assertions in
apps/extension/src/lib/sync.test.ts at lines 141-144 and 305-312: add await to
the expect(...).rejects.toThrow calls in the “refuses to run without an API key”
test and the server-reachability failure test, ensuring the later vault equality
assertion runs after syncVault settles.
In `@apps/extension/src/lib/sync.ts`:
- Around line 158-162: Validate the response from fetchExport before processing
it in the sync flow, using the shared authenticatorExportSchema from
packages/schemas; replace the unchecked IAuthenticatorExport cast with schema
parsing and use the parsed payload for remote.accounts and vault writes,
propagating validation failures instead of persisting malformed data.
- Around line 112-132: Update the successful create path in syncVault’s
pending-push loop to persist the updated entries immediately after each
createRemoteAccount succeeds, including the returned serverId and pendingPush
false state. Reuse the existing vault persistence mechanism so later sync
failures cannot lose the adopted server id and recreate the account.
In `@apps/extension/src/lib/vault.ts`:
- Around line 96-112: The write path in writeVault must reject stale or
unauthenticated session keys before persisting ciphertext. Validate that
currentKey successfully matches the current vault record’s
authentication/revision state (using the existing WrongPassphraseError or
revision/salt check) before calling writeVaultRecord, while preserving the
existing KDF reuse for valid writes and ensuring changePassphrase remains
protected by its current read/serialization flow.
In `@apps/extension/src/pages/options/options.tsx`:
- Around line 260-267: Update the onBlur handler to handle failures from
onCommit(parsed): await or otherwise catch the rejected promise and call
setDraft(String(value)) when the commit fails, while preserving the existing
validation reset and successful commit behavior.
- Around line 391-398: Defer URL revocation in the export flow after
anchor.click() by scheduling URL.revokeObjectURL(url) on the next event loop,
ensuring the blob remains available while the browser begins downloading. Keep
the existing download filename and click behavior unchanged.
In `@apps/extension/src/pages/popup/popup.tsx`:
- Around line 206-208: Update the onDelete handler in the popup entry action to
handle the promise returned by send for the deleteAccount request instead of
discarding it. Await or catch failures and route the error into the existing
failure state or user-facing error handling, while preserving the current
entry.id request payload.
- Around line 177-190: Update the search-controls render condition around the
Input to keep it visible when either entries.length exceeds five or search is
non-empty. Preserve the existing filtering behavior and allow users to clear an
active search after the account count drops to five or fewer.
- Around line 66-67: Update the entries selection in the popup component so
non-"unlocked" states reuse a stable empty-array reference instead of allocating
a new array on each render. Keep passing the unlocked payload entries to
useCodes, and define or reuse a module-level empty-array constant for the
fallback.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ebe73b95-01cc-4f3e-95d3-85176488249c
⛔ Files ignored due to path filters (3)
CLAUDE.mdis excluded by!*.mdREADME.mdis excluded by!*.mdbun.lockis excluded by!**/*.lock
📒 Files selected for processing (53)
.github/workflows/release-extension.ymlapps/extension/.gitignoreapps/extension/README.mdapps/extension/SOURCE.mdapps/extension/bunfig.tomlapps/extension/options.htmlapps/extension/package.jsonapps/extension/popup.htmlapps/extension/scripts/build.tsapps/extension/scripts/generate-icons.tsapps/extension/scripts/manifest.tsapps/extension/scripts/package.tsapps/extension/src/background/index.tsapps/extension/src/components/account-row.tsxapps/extension/src/components/add-account-dialog.tsxapps/extension/src/components/countdown-ring.tsxapps/extension/src/components/setup-screen.tsxapps/extension/src/components/unlock-screen.tsxapps/extension/src/global.d.tsapps/extension/src/hooks/use-codes.tsapps/extension/src/hooks/use-vault-state.tsapps/extension/src/lib/api.tsapps/extension/src/lib/browser.tsapps/extension/src/lib/clipboard.tsapps/extension/src/lib/crypto.test.tsapps/extension/src/lib/crypto.tsapps/extension/src/lib/entries.test.tsapps/extension/src/lib/entries.tsapps/extension/src/lib/format.tsapps/extension/src/lib/messages.tsapps/extension/src/lib/otpauth-uri.tsapps/extension/src/lib/session.tsapps/extension/src/lib/storage.tsapps/extension/src/lib/sync.test.tsapps/extension/src/lib/sync.tsapps/extension/src/lib/theme.tsapps/extension/src/lib/totp.test.tsapps/extension/src/lib/totp.tsapps/extension/src/lib/types.tsapps/extension/src/lib/vault.tsapps/extension/src/pages/options/main.tsxapps/extension/src/pages/options/options.tsxapps/extension/src/pages/popup/main.tsxapps/extension/src/pages/popup/popup.tsxapps/extension/src/styles/globals.cssapps/extension/store/listing.mdapps/extension/test-setup.tsapps/extension/tsconfig.jsonapps/extension/vite.config.tsapps/web/app/api/admin/authenticator/export/route.tsapps/web/app/extension-privacy/page.tsxapps/web/lib/authenticator.tspackages/schemas/src/authenticator.ts
| permissions: | ||
| contents: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow file =="
cat -n .github/workflows/release-extension.yml
echo
echo "== package/config references =="
fd -p 'package.json|bun.lockb|bun.lock|pnpm-lock.yaml|yarn.lock|x86' .github/workflows/release-extension.yml 2>/dev/null || true
echo
echo "== relevant checkouts/build hooks across repo =="
rg -n "bun(inst|run)|build|pre|post|prepare|install|hooks|lint|typecheck|verify|actions/checkout|persist-credentials|contents: write|permissions:" -S .github package.json tools scripts 2>/dev/null || trueRepository: denizlg24/denizlg24.com
Length of output: 19569
🌐 Web query:
actions/checkout persist-credentials default true commit GITHUB_TOKEN contents write permissions
💡 Result:
The actions/checkout action has a persist-credentials input that defaults to true [1][2]. When set to true, the authentication token (such as the GITHUB_TOKEN) used to check out the repository is persisted in the local Git configuration [1][3]. This allows subsequent steps in the same job to run authenticated Git commands without needing to manually re-authenticate [1][4]. The token is removed automatically during the post-job cleanup [1][3]. Regarding GITHUB_TOKEN permissions, while actions/checkout defaults to persisting these credentials [1][2], the actual capabilities available to those credentials depend on the permissions explicitly granted to the GITHUB_TOKEN within your workflow file [5][6]. The default GITHUB_TOKEN permissions for GitHub Actions workflows include only read access for repository contents (contents: read) [6]. To enable write operations (such as pushing code or creating branches), you must explicitly grant write permissions in your workflow configuration [6]: permissions: contents: write If you require a job to perform write actions, it is a recommended security practice to use the principle of least privilege by explicitly setting the required permissions in your workflow file rather than relying on default or broad access [5][6]. If your workflow only needs to clone or fetch the repository, keeping the default contents: read is sufficient [6]. To prevent the GITHUB_TOKEN from being persisted in the local Git config for security reasons, you can set persist-credentials to false in the checkout step [1][7].
Citations:
- 1: https://github.com/actions/checkout
- 2: https://github.com/actions/checkout/blob/cbb722410c2e876e24abbe8de2cc27693e501dcb/action.yml
- 3: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 4: https://github.com/actions/checkout/tree/v6.0.0
- 5: https://docs.github.com/actions/reference/authentication-in-a-workflow
- 6: https://adhdecode.com/articles/github-actions/github-actions-workflow-permissions/
- 7: Remove
persist-credentialsor change the default tofalseactions/checkout#485
Security Misconfiguration (CWE-250)
Scope contents: write to the release job and disable credential persistence in build steps.
The workflow-level contents: write grant also reaches prepare and build, whose checkouts persist the token by default. The build job already executes bun commands and repo scripts while that token is available, even without installing extra packages. Set the workflow default to contents: read, give release job-level write permissions, and add persist-credentials: false to the checkouts that only need repository access.
🧰 Tools
🪛 zizmor (1.28.0)
[error] 12-12: overly broad permissions (excessive-permissions): contents: write is overly broad at the workflow level
(excessive-permissions)
[warning] 12-12: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-extension.yml around lines 11 - 12, Update the
workflow-level permissions to contents: read, add contents: write specifically
to the release job, and set persist-credentials: false on checkout steps in
prepare and build that only require repository access. Keep the release
checkout’s credential behavior unchanged unless it does not need the write
token.
Source: Linters/SAST tools
| current_version="$(jq -r '.version' apps/extension/package.json)" | ||
|
|
||
| if [[ "${{ github.event_name }}" != "push" ]]; then | ||
| echo "changed=true" >> "$GITHUB_OUTPUT" | ||
| echo "tag=ext-v$current_version" >> "$GITHUB_OUTPUT" | ||
| echo "version=$current_version" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
|
|
||
| previous_version="$( | ||
| git show "${{ github.event.before }}:apps/extension/package.json" 2>/dev/null | | ||
| jq -r '.version' || | ||
| true | ||
| )" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Workflow around detected lines:\n'
sed -n '1,90p' .github/workflows/release-extension.yml
printf '\nSearch for release-extension reference and outputs:\n'
rg -n "release-extension|version|changed|tag=ext-v|previous_version|git show|github\.event\.before|github\.event_name" .github apps/extension -SRepository: denizlg24/denizlg24.com
Length of output: 18017
🌐 Web query:
GitHub Actions github.event.before push all-zero SHA zero commit
💡 Result:
In GitHub Actions, the github.event.before property is set to an all-zero SHA (0000000000000000000000000000000000000000) when there is no previous commit reference for the event [1]. This most commonly occurs during the first push to a new branch, the creation of a new tag, or following a force push [2][3][1]. Because this all-zero string is not a valid git object, workflows that attempt to use github.event.before directly in commands like git fetch or git diff will encounter fatal errors [4][2][3]. To handle this, developers commonly implement conditional logic to detect the null SHA and provide a fallback [3][5][6]. Common strategies include: 1. Skipping the command: If the operation is intended to compare changes between commits, skip the operation entirely when the before SHA is null, as there is no prior state to compare against [4][7]. 2. Using an empty tree: For git diff commands, substituting the null SHA with the git empty-tree SHA (4b825dc642cb6eb9a060e54bf8d69288fbee4904) can allow the diff to treat all files in the initial commit as newly added [2]. 3. Falling back to an alternative base: Some workflows fall back to a default branch (e.g., origin/main), the parent of the first commit in the push, or a recent commit timestamp to establish a base [3][5][8]. Example of defensive logic in a workflow: if [[ "${{ github.event.before }}" == "0000000000000000000000000000000000000000" ]]; then echo "First push detected, skipping base comparison." else git diff "${{ github.event.before }}" "${{ github.sha }}" fi
Citations:
- 1: https://stackoverflow.com/questions/61860732/how-can-i-get-the-previous-commit-before-a-push-or-merge-in-github-action-workfl
- 2: Fix lint job crash on tag/new-branch push (null SHA in git diff) tsaanghwang/Yime-prototype#23
- 3: loadingalias/cargo-rail-action@6254480
- 4: TT-17582: fix checkout-pr action failing on new branch push TykTechnologies/github-actions#139
- 5: [Feature]
since_last_remote_commitincorrecton: pushof new branch tj-actions/changed-files#387 - 6: https://github.com/trunk-io/trunk-action/blob/41ba2e0bf7ba011b28cb0c6755d359503dcb2e4e/push.sh
- 7: TT-17582: fix checkout-pr action failing on new branch push TykTechnologies/github-actions#139
- 8: https://github.com/AgoraIO/Docs-Source/blob/e6571b0a4713bad96f73542c2d0181c914aeed45/.github/workflows/inject-last-updated.yml
🌐 Web query:
GitHub Actions context payload github.event.before contains shell metacharacters
💡 Result:
Using GitHub Actions context variables—such as github.event.before—directly within run steps creates a security risk known as script or shell injection [1][2][3]. This occurs because GitHub Actions evaluates the ${{ }} expression and substitutes its raw value into your shell script before the shell executes it [1][4][5]. If an attacker can influence that value to contain shell metacharacters (e.g., quotes, semicolons, backticks, or subshell syntax), they can break out of the intended command string and execute arbitrary code on the runner [1][2][3]. While github.event.before is a SHA reference and therefore less likely to be directly manipulated by an attacker compared to fields like issue titles or pull request bodies, it is still considered a best practice to avoid direct interpolation for any context variable [6][7][5]. Security tools and hardening guides flag this pattern because it violates defensive programming principles [6][8][9]. To prevent shell injection, you should always avoid using the ${{ }} syntax directly in run commands [10][5][3]. Instead, pass the context value into an environment variable using the env: block [2][4][3]. Because environment variables are handled as data rather than script code, the shell will not interpret metacharacters within the variable's value [11][5][3]. Recommended pattern: # ❌ VULNERABLE: Direct interpolation run: git diff --name-only ${{ github.event.before }} ${{ github.sha }} # ✅ SECURE: Using environment variables env: BEFORE_SHA: ${{ github.event.before }} run: git diff --name-only "$BEFORE_SHA" "${{ github.sha }}" By assigning the value to an environment variable, the shell treats the content as a safe string [11][3]. Additionally, quoting the variable (e.g., "$BEFORE_SHA") is a standard shell best practice to prevent word splitting [10][5]. Automated security scanners such as CodeQL and zizmor are designed to detect unsafe inline interpolation and should be used to audit your workflows [2][10][6][7].
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://github.blog/security/vulnerability-research/how-to-catch-github-actions-workflow-injections-before-attackers-do/
- 3: https://orbisappsec.com/blog/github-actions-shell-injection-how-variables-can-hijack-your-cicd-pipeline
- 4: https://sebastion.dev/intelligence/2026-03-21-poc-github-actions-shell-injection-via-unsanitized-issue-metadata
- 5: https://securitylab.github.com/resources/github-actions-untrusted-input/
- 6: ci: fix expression injection pattern in l10n workflow git/git#2319
- 7: fix: template-injection: more safe contexts zizmorcore/zizmor#309
- 8: https://blog.devsecopsguides.com/p/github-actions-worm
- 9: https://git.joshthomas.dev/mirrors/zizmor/src/commit/c28d44c9032f0983829e217470d7c9b57804ddfe/src/audit/template_injection.rs
- 10: https://github.blog/security/supply-chain-security/four-tips-to-keep-your-github-actions-workflows-secure/
- 11: feat: auto-extract shell injection expressions from run: steps into env vars github/gh-aw#28998
🌐 Web query:
GitHub Actions github.event.before ref type SHA string
💡 Result:
In GitHub Actions, github.event.before is a property available within the github context for specific event types, most notably the push event [1][2]. It is a string representing the SHA (Secure Hash Algorithm) of the most recent commit on the reference (branch or tag) before the push occurred [3][4][2]. Key technical details regarding github.event.before: 1. Type and Format: It is a string containing a 40-character commit SHA [5][3]. 2. Usage: It is primarily used to determine the range of commits introduced by a push (i.e., the range between before and after [3][4][6]). 3. Null SHA Case: When a workflow is triggered by the creation of a new branch or tag, github.event.before is set to the "null SHA" (0000000000000000000000000000000000000000) [1][7]. Because this is not a valid git reference, attempting to use it directly in commands like git fetch or git diff without a conditional check can cause the workflow to fail [8][7][6]. 4. Best Practice: When using github.event.before in scripts, it is recommended to check if the value is the null SHA or empty to prevent errors during execution [7][6]. The github.event context mirrors the webhook payload for the event that triggered the workflow run [5][9][10]. Therefore, properties like before are specific to the structure of the push webhook payload and are not universal to all GitHub Actions events [5][9][3].
Citations:
- 1: https://stackoverflow.com/questions/61860732/how-can-i-get-the-previous-commit-before-a-push-or-merge-in-github-action-workfl
- 2: https://blog.dominicrodemer.com/github-actions-advanced-triggers-and-events/
- 3: https://docs.github.com/en/webhooks/webhook-events-and-payloads?actionType=fixed
- 4: https://github.com/github/gh-aw/blob/main/.github/aw/context.md
- 5: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 6: https://github.com/prantikng/sopm/blob/main/.github/workflows/validate-commit-messages.yml
- 7: TT-17582: fix checkout-pr action failing on new branch push TykTechnologies/github-actions#139
- 8: https://stackoverflow.com/questions/73905977/github-action-git-diff-could-not-access-current-sha-error
- 9: https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/contexts.md
- 10: https://github.com/github/docs/blob/main/content/actions/how-tos/write-workflows/choose-when-workflows-run/trigger-a-workflow.md
🌐 Web query:
GitHub push payload before SHA webhook before ref name
💡 Result:
In a GitHub push event webhook payload, the fields appear in the JSON object at the top level [1][2]. While JSON objects are technically unordered collections of key-value pairs, the standard schema for a push event includes the following core fields [1][3]: ref: The full Git reference that was pushed (e.g., refs/heads/main or refs/tags/v3.14.1) [1][4]. before: The SHA of the most recent commit on the ref before the push [1][5]. after: The SHA of the most recent commit on the ref after the push [1][3]. These fields are essential for identifying the state of the branch or tag before and after the specific push action occurred [1][6]. When receiving the payload, you can use these SHAs to compare the changes included in the push [1][3]. [1][2]
Citations:
- 1: https://docs.github.com/en/webhooks/webhook-events-and-payloads
- 2: https://www.fasthook.io/github-push-event-payload
- 3: https://docs.github.com/en/enterprise-server@3.17/webhooks-and-events/webhooks/webhook-events-and-payloads
- 4: https://docs.github.com/en/enterprise-cloud@latest/webhooks/webhook-events-and-payloads
- 5: https://docs.github.com/en/enterprise-server@3.21/webhooks/webhook-events-and-payloads
- 6: https://docs.github.com/en/enterprise-server@3.18/webhooks/webhook-events-and-payloads
Injection (CWE-94): Improper Control of Generation of Code ('Code Injection')
Pass event data through env, and handle the all-zero previous SHA.
-
${{ github.event_name }}and${{ github.event.before }}expand into the shell script before bash parses it. Assign them to env values and reference those shell variables in therunblock. - On the first push to a ref or after a force push,
github.event.beforecan be the all-zero SHA.git showthen fails, soprevious_versionbecomes empty and the workflow reportschanged=trueeven ifcurrent_versiondid not change. Treat an unresolvable previous commit as no version change.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 38-38: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 46-46: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-extension.yml around lines 36 - 49, Update the
workflow step containing current_version and previous_version to pass
github.event_name and github.event.before through env, then reference those
shell variables instead of embedding expressions in the run script. Handle an
all-zero or otherwise unresolvable previous SHA by treating the previous version
as the current version, so the comparison reports no version change when no
prior package version is available.
Source: Linters/SAST tools
| for (let row = 0; row < size; row++) { | ||
| raw[row * (stride + 1)] = 0; // filter type: none | ||
| Buffer.from(pixels.buffer, row * stride, stride).copy( | ||
| raw, | ||
| row * (stride + 1) + 1, | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Include pixels.byteOffset in the row views.
Buffer.from(pixels.buffer, ...) addresses the underlying ArrayBuffer, not the view. renderPixels returns a fresh array, so the offset is 0 now. If a caller later passes a subarray, every row reads from the wrong offset and the PNG output is silently corrupt.
♻️ Proposed change
- Buffer.from(pixels.buffer, row * stride, stride).copy(
- raw,
- row * (stride + 1) + 1,
- );
+ Buffer.from(
+ pixels.buffer,
+ pixels.byteOffset + row * stride,
+ stride,
+ ).copy(raw, row * (stride + 1) + 1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (let row = 0; row < size; row++) { | |
| raw[row * (stride + 1)] = 0; // filter type: none | |
| Buffer.from(pixels.buffer, row * stride, stride).copy( | |
| raw, | |
| row * (stride + 1) + 1, | |
| ); | |
| } | |
| for (let row = 0; row < size; row++) { | |
| raw[row * (stride + 1)] = 0; // filter type: none | |
| Buffer.from( | |
| pixels.buffer, | |
| pixels.byteOffset + row * stride, | |
| stride, | |
| ).copy(raw, row * (stride + 1) + 1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/scripts/generate-icons.ts` around lines 135 - 141, Update the
row buffer creation inside the icon-generation loop to add pixels.byteOffset to
the underlying-buffer offset before copying each row. Preserve the existing
stride and destination offsets so both full arrays and subarray views produce
correct PNG output.
| // Only the server this extension syncs with. Anything else the owner points | ||
| // it at is requested at runtime through optional_host_permissions. | ||
| host_permissions: [apiOrigin], | ||
| optional_host_permissions: ["https://*/*"], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In which Firefox version did optional_host_permissions become supported in manifest.json?
💡 Result:
Firefox introduced support for the optional_host_permissions manifest.json key in version 128 [1][2][3]. This key is intended for use with Manifest V3 or higher to specify host permissions that an extension can request at runtime [4][5][3]. While older versions of Firefox allowed host permissions to be defined within the optional_permissions key, the optional_host_permissions key is the current recommended standard [1][3].
Citations:
- 1: Warn about optional_permissions without optional_host_permissions / host_permissions mozilla/addons-linter#5329
- 2: https://github.com/mdn/content/blob/main/files/en-us/mozilla/firefox/releases/128/index.md?plain=1
- 3: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/optional_permissions
- 4: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/optional_host_permissions
- 5: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'apps/extension/scripts/(manifest|build)\.ts$|README|package\.json|firefox' || true
echo "== manifest outline =="
ast-grep outline apps/extension/scripts/manifest.ts || true
echo "== relevant manifest.ts lines =="
cat -n apps/extension/scripts/manifest.ts | sed -n '1,140p'
echo "== search for permissions.request / optional host usage =="
rg -n "permissions\.request|optional_host_permissions|optional_permissions|host_permissions" apps/extension -S || trueRepository: denizlg24/denizlg24.com
Length of output: 7200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== browser.ts outline =="
ast-grep outline apps/extension/src/lib/browser.ts || true
echo "== browser.ts relevant lines =="
cat -n apps/extension/src/lib/browser.ts | sed -n '1,120p'
echo "== build manifest call sites/options =="
rg -n "buildManifest|apiOrigin|optional_host_permissions|strict_min_version|browser_specific_settings" apps/extension -S || true
echo "== package scripts =="
cat apps/extension/package.json | jq '{scripts, extensions}' 2>/dev/null || trueRepository: denizlg24/denizlg24.com
Length of output: 4004
Align Firefox support versions with the optional permission key.
Firefox 128 is the first release that supports optional_host_permissions; Firefox 115.0–127.0 only supports optional hosts under optional_permissions. Keep the current floor only if the Firefox manifest also emits optional_permissions, otherwise raise browser_specific_settings.gecko.strict_min_version to "128.0".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/scripts/manifest.ts` at line 69, Align the Firefox minimum
version with the manifest’s optional_host_permissions key: either emit the
equivalent optional hosts through optional_permissions for Firefox 115.0–127.0,
or update browser_specific_settings.gecko.strict_min_version to "128.0" if
optional_host_permissions remains the sole key.
Source: Linters/SAST tools
| /** 1980-01-01, the earliest timestamp the ZIP format can store. Pinning it keeps | ||
| * the same build reproducible byte for byte. */ | ||
| const FIXED_MTIME = Date.UTC(1980, 0, 1); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
How does fflate zipSync interpret the mtime option and what are its valid date bounds?
💡 Result:
In the fflate library, the mtime option in zipSync allows you to specify the last modification time for files in a ZIP archive [1][2]. By default, fflate uses the current system time if this option is not provided [1][3]. The option accepts a string, a number (representing a Unix epoch timestamp), or a JavaScript Date object [1][2]. Regarding how it is interpreted and its constraints: 1. Local Timezone Encoding: The mtime is encoded into the ZIP file's DOS timestamp format using the Date object's local time components (e.g., getFullYear, getHours) [4][5]. This means that the exact byte representation in the ZIP archive may vary depending on the local timezone of the environment where the archive is created [4][5]. To ensure deterministic, reproducible builds regardless of the environment's timezone, it is recommended to provide a Date object explicitly constructed with specific local date components (e.g., new Date(2020, 0, 1)) rather than a UTC epoch timestamp [4][5]. 2. Valid Date Bounds: ZIP files use a legacy DOS format to store timestamps, which has a minimum representable date of January 1, 1980 [5]. Consequently, any constant date on or after this date is valid [5]. Dates prior to this epoch are not supported by the underlying ZIP specification's time field and should be avoided to ensure compatibility [5]. Top results: [4][5][6]
Citations:
- 1: https://github.com/101arrowz/fflate/blob/master/docs/interfaces/ZipOptions.md
- 2: https://github.com/101arrowz/fflate/blob/master/docs/interfaces/ZipAttributes.md
- 3: https://github.com/101arrowz/fflate/blob/master/docs/interfaces/ZipInputFile.md
- 4: Mtime is set incorrectly on timezon 101arrowz/fflate#219
- 5: https://github.com/appstrate/appstrate/blob/main/packages/core/src/zip.ts
- 6: https://github.com/101arrowz/fflate
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.ts =="
cat -n apps/extension/scripts/package.ts | sed -n '1,120p'
echo
echo "== package references =="
rg -n "FIXED_MTIME|zipSync|mtime" apps/extension/scripts apps -g '!node_modules' -g '!dist' || true
echo
echo "== lock/package deps fflate =="
if [ -f apps/extension/package.json ]; then
cat apps/extension/package.json | jq '.dependencies, .devDependencies' 2>/dev/null || cat apps/extension/package.json
fi
rg -n '"fflate"|"fflate@' package.json pnpm-lock.yaml yarn.lock bun.lockb bun.lock 2>/dev/null || true
echo
echo "== timezone behavior probe =="
node - <<'JS'
const offsetUTC = Date.UTC(1980, 0, 1);
const local = new Date(offsetUTC);
function dumpTZ(name, tz) {
process.env.TZ = tz;
const d = new Date(Date.UTC(1980, 0, 1));
const iso = d.toISOString();
const str = d.toString();
const minYear = d.getFullYear() < 1980;
const ms = d.getTime();
console.log(`${name} localDate=${str} ISO=${iso} yearBelow1980=${minYear}`);
}
for (const tz of ['UTC-5', 'Canada/Eastern', 'US/Pacific', 'UTC']) {
dumpTZ(tz);
}
JSRepository: denizlg24/denizlg24.com
Length of output: 7039
Use a local-time date for FIXED_MTIME.
fflate encodes mtime from the Date’s local fields into the ZIP DOS timestamp. Date.UTC(1980, 0, 1) represents a different local date in negative-offset timezones and will fall below ZIP’s 1980 minimum, breaking zipSync. Build a date with explicit local calendar fields so the release timestamp stays stable and valid everywhere.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/scripts/package.ts` around lines 27 - 29, Update the
FIXED_MTIME definition to construct a Date using explicit local calendar fields
for January 1, 1980, rather than Date.UTC. Preserve the fixed timestamp’s
reproducibility while ensuring fflate’s local-field encoding remains within the
ZIP format’s valid 1980 minimum in every timezone.
| onBlur={() => { | ||
| const parsed = Number(draft); | ||
| if (!Number.isFinite(parsed) || parsed < min || parsed > max) { | ||
| setDraft(String(value)); | ||
| return; | ||
| } | ||
| void onCommit(parsed); | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Surface a failed preference commit.
void onCommit(parsed) discards a rejected promise. If the background rejects updatePreferences, the input keeps showing the new value while the stored preference is unchanged. Revert the draft on failure.
♻️ Proposed refactor
- void onCommit(parsed);
+ void onCommit(parsed).catch(() => setDraft(String(value)));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onBlur={() => { | |
| const parsed = Number(draft); | |
| if (!Number.isFinite(parsed) || parsed < min || parsed > max) { | |
| setDraft(String(value)); | |
| return; | |
| } | |
| void onCommit(parsed); | |
| }} | |
| onBlur={() => { | |
| const parsed = Number(draft); | |
| if (!Number.isFinite(parsed) || parsed < min || parsed > max) { | |
| setDraft(String(value)); | |
| return; | |
| } | |
| void onCommit(parsed).catch(() => setDraft(String(value))); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/src/pages/options/options.tsx` around lines 260 - 267, Update
the onBlur handler to handle failures from onCommit(parsed): await or otherwise
catch the rejected promise and call setDraft(String(value)) when the commit
fails, while preserving the existing validation reset and successful commit
behavior.
| const url = URL.createObjectURL(blob); | ||
| const anchor = document.createElement("a"); | ||
| anchor.href = url; | ||
| anchor.download = `authenticator-vault-${new Date() | ||
| .toISOString() | ||
| .slice(0, 10)}.json`; | ||
| anchor.click(); | ||
| URL.revokeObjectURL(url); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does revoking a blob object URL immediately after anchor.click() cancel the download in Firefox?
💡 Result:
Yes, revoking a blob object URL immediately after calling anchor.click often cancels the download in Firefox [1][2][3]. The issue arises because the browser's download process is asynchronous [2][3][4]. When you trigger a download using anchor.click, the browser schedules the fetch operation for the blob. If you call URL.revokeObjectURL synchronously on the next line of code, you may revoke the object URL before the browser has actually started reading the data, causing the download to fail [2][3]. To avoid this, it is common practice to defer the revocation of the object URL using setTimeout(..., 0). This allows the JavaScript event loop to process the click initiation before the URL is revoked [2][5][6]. Example of the corrected pattern: const url = URL.createObjectURL(blob); const anchor = document.createElement('a'); anchor.href = url; anchor.download = 'filename.ext'; anchor.click; // Defer revocation to the next event loop task setTimeout( => URL.revokeObjectURL(url), 0); While the web standard implies that browsers should internally handle the blob reference safely once a navigation/download is triggered [1][4], browser implementations—including Firefox—have historically struggled with this determinism, making the use of a small timeout a standard, reliable workaround [1][2][7].
Citations:
- 1: https://bugzilla.mozilla.org/show_bug.cgi?id=1282407
- 2: Browser download revokes blob object-URL synchronously after click(), cancelling exports on Firefox/Safari Atypical-Consulting/Koine#623
- 3: https://stackoverflow.com/questions/54156588/revokeobjecturl-not-work-in-mozilla-firefox-and-microsoft-edge
- 4: Event to indicate download started and/or finished whatwg/html#954
- 5: https://github.com/receptron/mulmoclaude/blob/main/src/utils/blobDownload.ts
- 6: https://stackoverflow.com/questions/30694453/blob-createobjecturl-download-not-working-in-firefox-but-works-when-debugging
- 7: https://bugzilla.mozilla.org/show_bug.cgi?id=1800266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant options file around the reported lines.
if [ -f apps/extension/src/pages/options/options.tsx ]; then
sed -n '360,420p' apps/extension/src/pages/options/options.tsx | cat -n
else
echo "File not found: apps/extension/src/pages/options/options.tsx"
fd -a 'options\.tsx$' . | sed -n '1,20p'
fi
# Search for similar programmatic downloads to gauge repository patterns.
rg -n "URL\.createObjectURL|revokeObjectURL|anchor\.click\(\)" -S . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: denizlg24/denizlg24.com
Length of output: 5376
🌐 Web query:
MDN Document.body required for programmatic anchor click programmatic user activation
💡 Result:
There is no requirement to use document.body to programmatically trigger a user-activated anchor click. You can call the.click method directly on the specific HTMLAnchorElement [1][2]. To programmatically simulate a user interaction that satisfies browser security requirements (such as opening a popup or navigating), you should call the click method on the element itself [1][3]. This method is designed to simulate a real user click and will trigger the element's default activation behavior (e.g., following an href) if the element is not disabled [1][4]. Regarding the role of document.body: 1. Direct Triggering: Calling anchorElement.click is the standard and correct way to programmatically trigger an anchor [1][2]. 2. Event Delegation: Some developers use document.body as a target when setting up global event listeners for delegation [5] or when simulating clicks in testing environments (e.g., JSDOM or testing libraries) where a specific target element might not be available or fully supported [6][7]. 3. User Activation State: Browsers track "User Activation" (transient or sticky) based on actual user interaction [8][3]. Programmatically calling.click on an element generally does not grant "trusted" user activation status in the same way a physical mouse click or key press does [3]. Most modern browsers will block restricted actions (like window.open) if they are triggered by a script rather than a direct, hardware-based user event [3][9]. If your goal is to programmatically navigate or trigger behavior that typically requires user activation, simply calling.click on an anchor often will not bypass browser security restrictions meant to prevent abusive popups or unauthorized navigation [3].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/click
- 2: https://sqlpey.com/javascript/javascript-programmatic-click-methods/
- 3: https://developer.mozilla.org/en-US/docs/Web/Security/Defenses/User_activation
- 4: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/a
- 5: https://stackoverflow.com/questions/69209518/how-does-one-prevent-the-opening-of-a-new-tab-or-window-when-hyperlink-behavior
- 6: Add possibility to dispatch click on document testing-library/user-event#531
- 7: https://exchangetuts.com/simulating-click-on-document-reactjsjsdom-1640014803980593
- 8: https://developer.mozilla.org/en-US/docs/Web/API/UserActivation
- 9: Make "triggered by user activation" match browser behavior whatwg/html#1903
🌐 Web query:
MDN HTMLAnchorElement click programmatic click requirements body click download Firefox
💡 Result:
To programmatically trigger a download in Firefox using the HTMLAnchorElement.click method, it is required that the anchor element be present in the document DOM [1][2][3]. While simply creating an element with document.createElement("a") and calling.click may work in some other browsers, Firefox historically treats.click on an anchor element as a no-op if the element is not currently attached to the document body [1][2][4]. To successfully trigger the download, follow this pattern: 1. Create the anchor element: const link = document.createElement("a"); [2][3] 2. Configure it with the necessary attributes (e.g., href, download): link.href = url; link.download = "filename"; [2][3] 3. Append it to the document body: document.body.appendChild(link); [1][3] 4. Trigger the click: link.click; [1][2][3] 5. Remove it from the DOM immediately after: document.body.removeChild(link); (or link.remove;) [1][3][4] Note that using dispatchEvent to simulate a click (e.g., dispatching a MouseEvent) is generally not sufficient to trigger the default navigation or download behavior for anchor elements, as browsers restrict synthetic events to prevent unauthorized actions [5]. The.click method is the standard way to trigger these default actions, provided the DOM requirement is met [1][5].
Citations:
- 1: https://bugzilla.mozilla.org/show_bug.cgi?id=1218456
- 2: https://bugzilla.mozilla.org/show_bug.cgi?id=1091035
- 3: https://stackoverflow.com/questions/55162944/document-createelementa-click-not-working-in-firefox
- 4: https://stackoverflow.com/questions/35469943/anchor-click-to-download-not-working-in-firefox
- 5: https://bugzilla.mozilla.org/show_bug.cgi?id=395917
Defer the blob URL revocation after the download starts.
anchor.click() returns before the browser has fully fetched the blob, so URL.revokeObjectURL(url) can revoke the backing URL too early and cancel the export in Firefox. Trigger the click, then revoke the URL in the next event loop.
🐛 Proposed fix
const url = URL.createObjectURL(blob);
const anchor = document.createElement("a");
anchor.href = url;
anchor.download = `authenticator-vault-${new Date()
.toISOString()
.slice(0, 10)}.json`;
- anchor.click();
- URL.revokeObjectURL(url);
+ anchor.click();
+ setTimeout(() => URL.revokeObjectURL(url), 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const url = URL.createObjectURL(blob); | |
| const anchor = document.createElement("a"); | |
| anchor.href = url; | |
| anchor.download = `authenticator-vault-${new Date() | |
| .toISOString() | |
| .slice(0, 10)}.json`; | |
| anchor.click(); | |
| URL.revokeObjectURL(url); | |
| const url = URL.createObjectURL(blob); | |
| const anchor = document.createElement("a"); | |
| anchor.href = url; | |
| anchor.download = `authenticator-vault-${new Date() | |
| .toISOString() | |
| .slice(0, 10)}.json`; | |
| anchor.click(); | |
| setTimeout(() => URL.revokeObjectURL(url), 0); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/src/pages/options/options.tsx` around lines 391 - 398, Defer
URL revocation in the export flow after anchor.click() by scheduling
URL.revokeObjectURL(url) on the next event loop, ensuring the blob remains
available while the browser begins downloading. Keep the existing download
filename and click behavior unchanged.
| const entries = state.status === "unlocked" ? state.payload.entries : []; | ||
| const codes = useCodes(entries); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Prevent the non-unlocked render loop.
When state.status is not "unlocked", Line 66 creates a new empty array on every render. useCodes observes that new identity on Lines 33-36 of apps/extension/src/hooks/use-codes.ts and writes a new CodeMap. This schedules another render indefinitely. The loading, setup, and locked screens can hit React’s update-depth guard. Reuse a stable empty array.
Proposed fix
import type { VaultEntry } from "../../lib/types";
+const EMPTY_ENTRIES: VaultEntry[] = [];
+
function PopupShell({ children }: { children: React.ReactNode }) {
@@
- const entries = state.status === "unlocked" ? state.payload.entries : [];
+ const entries =
+ state.status === "unlocked" ? state.payload.entries : EMPTY_ENTRIES;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const entries = state.status === "unlocked" ? state.payload.entries : []; | |
| const codes = useCodes(entries); | |
| const entries = | |
| state.status === "unlocked" ? state.payload.entries : EMPTY_ENTRIES; | |
| const codes = useCodes(entries); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/src/pages/popup/popup.tsx` around lines 66 - 67, Update the
entries selection in the popup component so non-"unlocked" states reuse a stable
empty-array reference instead of allocating a new array on each render. Keep
passing the unlocked payload entries to useCodes, and define or reuse a
module-level empty-array constant for the fallback.
| {entries.length > 5 && ( | ||
| <div className="px-3 py-2 border-b border-border/50 shrink-0"> | ||
| <div className="relative"> | ||
| <Search className="absolute left-2.5 top-1/2 -translate-y-1/2 size-3.5 text-muted-foreground" /> | ||
| <Input | ||
| value={search} | ||
| onChange={(event) => setSearch(event.target.value)} | ||
| placeholder="Search" | ||
| aria-label="Search accounts" | ||
| className="pl-8 h-7 text-xs" | ||
| /> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep an active search controllable.
If the account count drops from six to five while a search is active, this condition removes the input. The retained search value still filters filtered, so nonmatching accounts stay hidden with no visible way to clear the filter. Keep the input visible while search is non-empty.
Proposed fix
- {entries.length > 5 && (
+ {(entries.length > 5 || search.length > 0) && (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {entries.length > 5 && ( | |
| <div className="px-3 py-2 border-b border-border/50 shrink-0"> | |
| <div className="relative"> | |
| <Search className="absolute left-2.5 top-1/2 -translate-y-1/2 size-3.5 text-muted-foreground" /> | |
| <Input | |
| value={search} | |
| onChange={(event) => setSearch(event.target.value)} | |
| placeholder="Search" | |
| aria-label="Search accounts" | |
| className="pl-8 h-7 text-xs" | |
| /> | |
| </div> | |
| </div> | |
| )} | |
| {(entries.length > 5 || search.length > 0) && ( | |
| <div className="px-3 py-2 border-b border-border/50 shrink-0"> | |
| <div className="relative"> | |
| <Search className="absolute left-2.5 top-1/2 -translate-y-1/2 size-3.5 text-muted-foreground" /> | |
| <Input | |
| value={search} | |
| onChange={(event) => setSearch(event.target.value)} | |
| placeholder="Search" | |
| aria-label="Search accounts" | |
| className="pl-8 h-7 text-xs" | |
| /> | |
| </div> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/src/pages/popup/popup.tsx` around lines 177 - 190, Update the
search-controls render condition around the Input to keep it visible when either
entries.length exceeds five or search is non-empty. Preserve the existing
filtering behavior and allow users to clear an active search after the account
count drops to five or fewer.
| onDelete={() => | ||
| void send({ type: "deleteAccount", id: entry.id }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle rejected delete requests.
send rejects when the background does not respond or returns an error. Line 206 discards that promise. A failed delete therefore creates an unhandled promise rejection. Await the request and store or display its failure state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/src/pages/popup/popup.tsx` around lines 206 - 208, Update the
onDelete handler in the popup entry action to handle the promise returned by
send for the deleteAccount request instead of discarding it. Await or catch
failures and route the error into the existing failure state or user-facing
error handling, while preserving the current entry.id request payload.
1 participant
Codes are generated locally from an encrypted vault instead of being polled, so they keep working when the server does not. Secrets come from a new admin-only export route, are sealed with AES-256-GCM under a passphrase-derived key, and accounts added in the extension sync back upstream.
Server-side deletions mirror into a local trash rather than dropping the secret outright, and a release workflow builds store-submittable packages whenever the extension version changes on main.
Summary by CodeRabbit
New Features
Documentation