Dev to master #61
Open
Alexsanchez06 wants to merge 34 commits into
Open
Conversation
latest binary v7.0.0 updated
…ted labels in myBns.
…keup Fix wallet reconnect after wakeup
Member
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughUpgrades the app to Node 24/Electron with context isolation enabled, a new preload script that exposes a channel-restricted ChangesElectron Security Hardening, Node Upgrade, and Renderer Refactor
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/menus/wallet_settings.vue (1)
525-536: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore a real path joiner for the key-image defaults.
parts.join("/")drops the normalization that the previous join helper provided. These values are passed straight to key-image import/export, so Windows and separator-edge cases can end up with broken paths. Keep using a platform-aware join here.🛠️ Suggested fix
+import upath from "upath"; ... - const joinPath = (...parts) => parts.filter(Boolean).join("/"); + const joinPath = (...parts) => upath.join(...parts.filter(Boolean));🤖 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 `@src/components/menus/wallet_settings.vue` around lines 525 - 536, The key-image default paths in wallet_settings.vue are now built with a string joiner, which loses platform-aware path normalization. Update the logic around the key_image export_path and import_path assignment to use the existing real path join helper or the appropriate platform-aware join function instead of a manual "/"-based join, so key-image import/export works correctly across separators and Windows paths.src-electron/main-process/modules/wallet-rpc.js (1)
3053-3061: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSanitize the Axios error before returning it
error.cause || errorcan carry the full Axios error object, including request config/auth, into the wallet gateway response. Return only a minimal sanitized cause (code,message,status) instead.🤖 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 `@src-electron/main-process/modules/wallet-rpc.js` around lines 3053 - 3061, The wallet-rpc error handler is returning the full Axios error object via error.cause || error, which can leak sensitive request details into the gateway response. Update the catch block in the wallet RPC flow to sanitize the cause before returning it, keeping only a minimal object with code, message, and status. Use the existing error handling around the method/params response shape in wallet-rpc.js to preserve the current structure while replacing the unsafe cause payload.
🟡 Minor comments (21)
build/entitlements.mac.plist-5-10 (1)
5-10: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winRemove the extra hardened-runtime exceptions
allow-jitis expected for Electron, butallow-unsigned-executable-memoryanddisable-library-validationare broad exceptions and should stay out unless a specific library or plugin needs them.🤖 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 `@build/entitlements.mac.plist` around lines 5 - 10, Remove the unnecessary hardened-runtime entitlements from entitlements.mac.plist, keeping only the Electron-required allow-jit exception. Update the entitlement set so com.apple.security.cs.allow-unsigned-executable-memory and com.apple.security.cs.disable-library-validation are omitted unless a specific runtime dependency in the app actually requires them.src/css/app.styl-934-940 (1)
934-940: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMatch the selector to the component class.
Line 934 defines
.dropDown, butsrc/pages/wallet/currencyDropDown.vuerendersclass="dropdown". These styles never apply unless the selector or template casing is aligned.Suggested fix
-.dropDown +.dropdown &:hover background-color `#242433` width 100% border-radius 8px cursor pointer🤖 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 `@src/css/app.styl` around lines 934 - 940, The dropdown hover styles are targeting the wrong class name, so they do not apply to the rendered component. Update the selector in app.styl or the class binding in currencyDropDown.vue so the stylesheet and the template use the same class name, and make sure the .dropDown rule matches the actual dropdown element rendered by the component.src/css/app.styl-1282-1292 (1)
1282-1292: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winComplete the hover selectors for the new button variants.
Line 1282 skips
.addressbook-btn:hover, and Line 1292 omits.active > .transaction-send-btn:hoverwhile also using.active > .addressbook-btnwithout:hover. That leaves the new buttons with inconsistent hover behavior versus the other right-pane actions.Suggested fix
- .send-btn:hover,.transaction-send-btn:hover, .contact-btn:hover, .receive-btn:hover + .send-btn:hover, .transaction-send-btn:hover, .contact-btn:hover, .receive-btn:hover, .addressbook-btn:hover background-color `#484866` - .active > .send-btn:hover, .active >.contact-btn:hover, .active > .receive-btn:hover,.active > .addressbook-btn + .active > .send-btn:hover, .active > .transaction-send-btn:hover, .active > .contact-btn:hover, .active > .receive-btn:hover, .active > .addressbook-btn:hover background-color `#478EFF`🤖 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 `@src/css/app.styl` around lines 1282 - 1292, The hover selector set in app.styl is incomplete for the new button variants. Update the shared hover rules around the send/transaction/contact/receive/addressbook buttons so .addressbook-btn:hover is included alongside the other base hover selectors, and ensure the active-state hover selector in the same block includes .active > .transaction-send-btn:hover and uses .active > .addressbook-btn:hover consistently. Keep the changes localized to the existing button selector groups so the hover behavior matches across all right-pane actions.src/i18n/de.js-69-71 (1)
69-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the verb form for the shared Open button.
"Offen"is an adjective, so generic dialogs will render the wrong action label here. This should be the imperative form used for opening.🤖 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 `@src/i18n/de.js` around lines 69 - 71, The shared Open action label in the German i18n dictionary is using the adjective form instead of the imperative verb form. Update the open entry in the translation object so generic dialogs display the correct action text, and keep the change localized to the relevant key in the i18n dictionary.src/i18n/de.js-187-191 (1)
187-191: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFinish localizing these German strings.
This span still mixes English and Russian into the
decatalog (sweepAllWarning.message,invalidNameFormat,masterNodeStartStakingDescription1,noMasterNodesCurrentlyAvailable). German users will see mixed-language dialogs and validation copy on these paths.Also applies to: 424-426, 638-642
🤖 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 `@src/i18n/de.js` around lines 187 - 191, The German locale still contains untranslated mixed-language strings in the `de` catalog, including `sweepAllWarning.message`, `invalidNameFormat`, `masterNodeStartStakingDescription1`, and `noMasterNodesCurrentlyAvailable`. Update these entries in `src/i18n/de.js` so they are fully localized to German, matching the surrounding keys and preserving the same meaning for the dialogs and validation text.src/i18n/en-us.js-248-257 (1)
248-257: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep
decryptRecordmapped to a decrypt label.
fieldLabels.decryptRecordnow says"Add Record", but this file already introduces a dedicatedstrings.bns.addRecordkey for that action. Reusing the older key changes the meaning of any existingdecryptRecordcaller and will surface the wrong label anywhere the decrypt flow still uses it.Also applies to: 579-586
🤖 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 `@src/i18n/en-us.js` around lines 248 - 257, The `decryptRecord` label in `en-us.js` was changed to the add-record text, which breaks existing decrypt-flow callers. Update the `fieldLabels.decryptRecord` entry to keep the decrypt-specific label, and use the existing `strings.bns.addRecord` key only for the add-record action; make the same correction anywhere the `decryptRecord` mapping was updated elsewhere in this file so `decryptRecord` and `addRecord` stay distinct.src/i18n/es.js-220-223 (1)
220-223: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the typo in the Spanish renew confirmation label.
RENVARwill be shown verbatim on the dialog button. This should beRENOVAR.🤖 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 `@src/i18n/es.js` around lines 220 - 223, The Spanish renew confirmation label in confirmRenew has a typo: the ok text currently shows “RENVAR” and should be corrected to “RENOVAR”. Update the translation entry in the es.js i18n object so the dialog button displays the proper Spanish verb.src/components/bns/bns_record_list.vue-199-202 (1)
199-202: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDon’t expose
bnsRenewas the image alt text.Screen readers will announce the literal token here. Since the button already has visible text, make the icon decorative (
alt="") or localize it with the renew label instead.🤖 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 `@src/components/bns/bns_record_list.vue` around lines 199 - 202, The renew button icon in bns_record_list.vue is exposing the literal bnsRenew token through its alt text. Update the image used alongside the $t("buttons.renew") label so it is decorative with an empty alt, or use the same localized renew label for the alt text, and keep the change in the template where the renew button/icon is rendered.src/i18n/fr.js-267-270 (1)
267-270: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
fieldLabels.bnsTypeis translated to the wrong concept.It currently renders
"Propriétaire de la sauvegarde", which is the backup-owner label, not the BNS record type. That will mislabel the field in the form.🤖 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 `@src/i18n/fr.js` around lines 267 - 270, The field label for fieldLabels.bnsType in the French locale is mapped to the wrong concept, so update the translation in fr.js to reflect the BNS record type rather than the backup-owner label. Use the existing fieldLabels object entry for bnsType and replace the current text with the correct French wording for BNS type, keeping the surrounding locale keys like localDaemonIP and belnetFullAddress unchanged.src/i18n/fr.js-229-239 (1)
229-239: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFrench unlock flow still shows a Spanish action label.
ok: "Desbloquear"will leak Spanish in the French unlock-master-node dialog.🤖 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 `@src/i18n/fr.js` around lines 229 - 239, The French unlock-master-node dialog still has a Spanish action label in the unlockMasterNode object. Update the ok value in fr.js to use the correct French text so the unlockMasterNode translation is fully localized, and verify the related unlockMasterNodeWarning entry remains consistent with the surrounding French strings.src/components/bns/bns_input.vue-203-205 (1)
203-205: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPlural years still bypass i18n.
The
content > 1branch hardcodes"years", so localized dialogs will still show English for values like2 years. Both branches need to come from translation keys/pluralization.🤖 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 `@src/components/bns/bns_input.vue` around lines 203 - 205, The year label logic in bns_input.vue still hardcodes the plural string in the content handling branch, so localized output can fall back to English. Update the content formatting logic near the existing content ternary to use translation keys or pluralization through this.$t for both singular and plural forms, and keep the same behavior in the surrounding formatter so all year text is fully i18n-driven.src/i18n/es.js-425-429 (1)
425-429: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe Spanish name-validation copy is self-contradictory.
invalidNameFormatsays only alphanumerics and hyphens are allowed, butinvalidNameHypenNotAllowedthen says underscores are valid at the edges. Users will get conflicting instructions while fixing the same error.🤖 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 `@src/i18n/es.js` around lines 425 - 429, The Spanish name-validation strings are inconsistent: invalidNameFormat and invalidNameHypenNotAllowed currently describe conflicting allowed characters. Update the copy in es.js so both messages use the same naming rules, and make sure invalidNameHypenNotAllowed matches the actual validation logic around edge characters and any underscore/hyphen handling.src/i18n/pt-br.js-563-566 (1)
563-566: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate these new pt-BR values before shipping.
These entries still contain English copy, so the UI will switch languages inside the same BNS flow.
🤖 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 `@src/i18n/pt-br.js` around lines 563 - 566, The new pt-BR strings in the i18n mapping still use English copy, so translate the `encryptedBelnetValue`, `encryptedWalletValue`, and `encryptedEthAddrValue` entries to Brazilian Portuguese to match `encryptedBchatValue` and keep the BNS flow fully localized.src/i18n/ru.js-218-220 (1)
218-220: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse renew wording in
dialog.confirmRenew.These values currently say “update”, so the renew confirmation dialog shows the wrong action text.
🤖 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 `@src/i18n/ru.js` around lines 218 - 220, The dialog.confirmRenew copy is using “update” wording instead of renew wording, so the confirmation dialog shows the wrong action text. Update the confirmRenew translation entries in the ru locale object to use renew-specific phrasing, keeping the existing dialog.confirmRenew key and its title/ok fields consistent with renewal terminology.src/i18n/ru.js-365-366 (1)
365-366: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReplace the leftover non-Russian strings in this locale file.
These entries are still Portuguese or English, which will make the Russian UI jump languages in success, error, and status messages.
Also applies to: 410-410, 518-518
🤖 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 `@src/i18n/ru.js` around lines 365 - 366, Replace the leftover non-Russian strings in the ru locale file so the Russian UI stays consistent. Update the affected translation entries, including decryptedBNSRecord and the other noted strings around the same locale object, to proper Russian text instead of Portuguese or English. Use the existing keys in src/i18n/ru.js and keep the wording aligned with nearby Russian messages in the same translation map.src/i18n/ru.js-126-126 (1)
126-126: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the interpolation token name consistent.
This title uses
{тип}, but the surrounding locale keys use{type}. If the dialog code passestype, the Russian title will render the raw placeholder.🤖 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 `@src/i18n/ru.js` at line 126, The Russian translation title uses a mismatched interpolation token, so update the title in the locale entry to use the same placeholder name as the rest of the i18n strings. In the translation object in ru.js, keep the token consistent with the surrounding keys by replacing the non-English placeholder in the title with the standard one used by the dialog code, so the value passed as type renders correctly.src/i18n/pt-br.js-12-12 (1)
12-12: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the pt-BR action labels here.
buttons.clear,buttons.decrypt, andbuttons.signare currently translated as “Claro”, “DESCRIPTO”, and “Sinal”, which change the intended action meaning in the UI.Also applies to: 21-21, 54-54
🤖 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 `@src/i18n/pt-br.js` at line 12, The pt-BR action labels in the buttons translation object are incorrect and should preserve the intended UI actions. Update the entries for buttons.clear, buttons.decrypt, and buttons.sign in the pt-br locale module so they use proper Portuguese action labels instead of meanings like “Claro”, “DESCRIPTO”, and “Sinal”. Locate the fixes in the buttons section of the locale object where these keys are defined and align them with the rest of the translations.src/components/address_book_details.vue-444-453 (1)
444-453: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the discard message key for the dialog body.
Line 445 points
messageatdialog.discardEdit.title, so the modal shows the title twice and drops the localized warning text.Proposed fix
.dialog({ title: this.$t("dialog.discardEdit.title"), - message: this.$t("dialog.discardEdit.title"), + message: this.$t("dialog.discardEdit.message"), ok: { label: this.$t("dialog.discardEdit.ok"), color: "primary"🤖 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 `@src/components/address_book_details.vue` around lines 444 - 453, The discard confirmation dialog in address_book_details.vue is using the title translation for both the heading and body, so the warning text is missing. Update the dialog configuration in the discard-edit flow to keep the title on dialog.discardEdit.title but point the message field to the discard message translation key used for the body, leaving the ok/cancel labels unchanged.src/pages/wallet/swap.vue-692-694 (1)
692-694: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the same interpolation key as the locale message.
placeholders.enterRecipientAddressis defined with{coin}, but this call passestype, so translated placeholders render the raw token instead of the currency name.Proposed fix
- this.$t('placeholders.enterRecipientAddress', { - type: this.sendAmounType.name - }) + this.$t('placeholders.enterRecipientAddress', { + coin: this.sendAmounType.name + })🤖 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 `@src/pages/wallet/swap.vue` around lines 692 - 694, The placeholder interpolation in the recipient address message uses the wrong key, so the locale token is not replaced correctly. Update the call in the swap view where this.$t('placeholders.enterRecipientAddress', ...) is used to pass the same interpolation key expected by the locale message, matching the {coin} placeholder instead of type. Make the change in the recipient address placeholder logic near the sendAmounType usage so translated text renders the currency name properly.src/components/bns/bns_update_input_form.vue-55-57 (1)
55-57: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFinish localizing the rest of this selector.
Line 55 starts the i18n migration, but the sibling BChat/Belnet/ETH labels and several placeholder fallbacks in the same block are still hardcoded English. In non-English locales this form will render mixed-language content.
🤖 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 `@src/components/bns/bns_update_input_form.vue` around lines 55 - 57, The selector in bns_update_input_form.vue is only partially localized, leaving the BChat, Belnet, ETH labels and nearby placeholder fallback text hardcoded in English. Update the same template block to use the existing i18n pattern via $t for every visible label and fallback string, including the sibling option labels and any placeholder text, so the whole selector renders consistently in all locales.src-electron/main-process/modules/backend.js-426-428 (1)
426-428: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winPrefer HTTPS-only external URLs.
open_urlis renderer-triggered, so allowinghttp:still permits plaintext external navigation. Unless a specific HTTP allowlist is required, restrict this guard tohttps:.Proposed fix
- return parsedUrl.protocol === "https:" || parsedUrl.protocol === "http:"; + return parsedUrl.protocol === "https:";Also applies to: 871-878
🤖 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 `@src-electron/main-process/modules/backend.js` around lines 426 - 428, The external URL handling in backend.js currently allows renderer-triggered navigation for any URL accepted by isSafeExternalUrl(), including plaintext http links. Tighten the guard in the open_url path so shell.openExternal is only reached for https:// URLs, and update any related URL validation used in the additional open_url handling block referenced by the same logic. Keep the change localized to the URL-safety check so the existing open_external flow still works for secure links while rejecting non-HTTPS schemes.
🧹 Nitpick comments (2)
package.json (1)
31-31: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRemove the npm
httpsdependency frompackage.jsonandpackage-lock.json.src-electron/main-process/modules/swap.jsalready uses Node’s corehttpsmodule (require("https")), so this package entry is redundant and adds avoidable supply-chain risk.🤖 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 `@package.json` at line 31, Remove the redundant npm https dependency from the project manifest and lockfile. Update package.json to drop the https package entry, and make sure package-lock.json no longer references that package. The relevant usage is already covered by the Node core https module in src-electron/main-process/modules/swap.js, so no code changes are needed there.src/components/menus/mainmenu.vue (1)
87-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid composing translated version labels from fragments.
Lines 87-91 hardcode the English order as
Wallet/Daemon + $t("strings.version") + ":". That won’t localize cleanly in languages that need a different noun/label order. Please switch these to full-sentence i18n keys with interpolation instead of concatenation.🤖 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 `@src/components/menus/mainmenu.vue` around lines 87 - 91, The version labels in mainmenu.vue are built by concatenating English fragments with this.$t("strings.version"), which prevents proper localization order. Update the Wallet and Daemon labels to use dedicated full-sentence i18n keys with interpolation, and render them through the existing template section so the noun, version term, and punctuation can be translated in the correct order.
🤖 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 `@src-electron/main-process/electron-main.js`:
- Around line 53-58: The renderer CSP in electron-main.js still whitelists
https://api.changelly.com, which bypasses the new main-process request path.
Remove Changelly from renderer connect-src in rendererConnectSrc unless there is
a deliberate remaining renderer call, and keep those swap requests routed
through IPC/backend via the main-process path instead.
- Around line 231-240: The powerMonitor handlers in electron-main.js should
guard against sending IPC before the app is ready. Update the suspend/resume
listeners so they only call mainWindow.webContents.send when mainWindow and
mainWindow.webContents exist and the window is not destroyed, and make the
resume path skip or defer appResumed unless startingToken is present. Keep the
fix localized to the powerMonitor.on("suspend") and powerMonitor.on("resume")
flow so the gateway always receives a valid config.token.
In `@src-electron/main-process/electron-preload.js`:
- Around line 10-27: The cleanArgsForIPC redactValue helper only redacts based
on key names, so sensitive substrings inside nested string values can still
reach the logger. Update redactValue in electron-preload.js to apply the same
redaction rules to string leaf values as well as to objects and arrays, and keep
using cleanArgsForIPC where the sanitized args are logged. Use the existing
redactKeyPattern and redactValue symbols so nested values like message fields
are redacted before logging.
- Around line 216-218: The shell.openExternal bridge in electron-preload.js
forwards any URL directly from the renderer, bypassing the main-process
isSafeExternalUrl() check. Update the openExternal handler to validate the
incoming url against the same allowed external protocols before calling
shell.openExternal, and reject or no-op for anything else. Keep the fix
localized to the shell.openExternal path so the preload only exposes the
already-approved navigation behavior.
In `@src-electron/main-process/logging.js`:
- Around line 30-46: The redaction logic in redactValue only hides sensitive
data when the object key matches, so nested string values like message fields
can still leak secrets. Update redactValue in logging.js to also inspect string
values recursively (and redact or mask them when they contain sensitive
labels/patterns, not just when the key matches), while preserving the existing
behavior for arrays and objects. Keep the fix localized to redactKeyPattern and
redactValue so all logged payloads are sanitized consistently.
In `@src-electron/main-process/modules/backend.js`:
- Around line 341-346: The settings save flow in Backend’s writeConfig callbacks
ignores the error argument, so failures still trigger set_app_data or app
startup. Update the save_config_init and save_config paths in
Backend.writeConfig to check the callback error first, and only acknowledge the
save or continue startup when writeConfig succeeds. Keep the existing
send("set_app_data", ...) and startup logic behind that success guard so failed
serialization/encryption/write attempts are not treated as persisted.
In `@src-electron/main-process/modules/swap.js`:
- Around line 334-336: The swap transport error handling in the sendRPC catch
block is leaking sensitive Axios request data. Update the catch path in swap.js
so the sendRPC error is logged with only redacted metadata instead of the raw
err object, and make sure the function returns a sanitized error object rather
than the original exception. Use the existing sendRPC catch and swap transport
error handling flow to locate the fix, and ensure any request config or headers
such as x-api-key / X-Api-Key are stripped before logging or returning.
In `@src-electron/main-process/modules/wallet-rpc.js`:
- Around line 2061-2063: The update flow in updateBNSMapping is using the wrong
RPC field name for the backup owner, so the change is likely ignored. Update the
params assignment in updateBNSMapping to use the RPC’s backup_owner key,
matching the naming already used by purchaseBNS, and keep the rest of the
request shape unchanged.
- Around line 532-539: The synchronous PBKDF2 call in derivePasswordHashSync is
blocking the Electron main process and can freeze wallet flows; update the
wallet-rpc.js password hashing path to use the async derivePasswordHash helper
instead of crypto.pbkdf2Sync, and adjust the call sites that
open/create/restore/import/change passwords so they await the asynchronous
result.
In `@src/components/footer.vue`:
- Line 101: The syncing check in footer.vue uses `this.target_height` for
`isSyncing`, which breaks `local_remote` because `target_height()` resolves to
`daemon.info.height` and makes the comparison always false. Update the
`isSyncing` logic in the footer component to use the same height source
consistently for `local_remote`, following the existing `target_height()`/daemon
height handling in the component so the footer stays in syncing state until the
local daemon actually catches up.
In `@src/index.template.html`:
- Around line 8-13: The production Content-Security-Policy in the
index.template.html meta tag still allows https://api.changelly.com, which
leaves the renderer CSP looser than the tightened main-process policy. Update
the CSP string in the template so it matches the renderer’s pinned/signed swap
path policy by removing the Changelly endpoint from connect-src, and keep the
rest of the meta CSP aligned with the symbols/condition around ctx.prod in the
template.
In `@src/layouts/wallet/rightPane.vue`:
- Around line 19-29: The send/receive controls in rightPane.vue are implemented
as a div, so the disable state on the transaction-send-btn is ineffective and
the action is not keyboard-accessible. Update the send/receive control markup in
the sendAndReceiveBtn section to use an actual button-based component or
element, and wire the disabled state to view_only so it cannot be activated in
view-only wallets. Keep the existing router('send') behavior and preserve the
active-state class logic tied to this.routes.
---
Outside diff comments:
In `@src-electron/main-process/modules/wallet-rpc.js`:
- Around line 3053-3061: The wallet-rpc error handler is returning the full
Axios error object via error.cause || error, which can leak sensitive request
details into the gateway response. Update the catch block in the wallet RPC flow
to sanitize the cause before returning it, keeping only a minimal object with
code, message, and status. Use the existing error handling around the
method/params response shape in wallet-rpc.js to preserve the current structure
while replacing the unsafe cause payload.
In `@src/components/menus/wallet_settings.vue`:
- Around line 525-536: The key-image default paths in wallet_settings.vue are
now built with a string joiner, which loses platform-aware path normalization.
Update the logic around the key_image export_path and import_path assignment to
use the existing real path join helper or the appropriate platform-aware join
function instead of a manual "/"-based join, so key-image import/export works
correctly across separators and Windows paths.
---
Minor comments:
In `@build/entitlements.mac.plist`:
- Around line 5-10: Remove the unnecessary hardened-runtime entitlements from
entitlements.mac.plist, keeping only the Electron-required allow-jit exception.
Update the entitlement set so
com.apple.security.cs.allow-unsigned-executable-memory and
com.apple.security.cs.disable-library-validation are omitted unless a specific
runtime dependency in the app actually requires them.
In `@src-electron/main-process/modules/backend.js`:
- Around line 426-428: The external URL handling in backend.js currently allows
renderer-triggered navigation for any URL accepted by isSafeExternalUrl(),
including plaintext http links. Tighten the guard in the open_url path so
shell.openExternal is only reached for https:// URLs, and update any related URL
validation used in the additional open_url handling block referenced by the same
logic. Keep the change localized to the URL-safety check so the existing
open_external flow still works for secure links while rejecting non-HTTPS
schemes.
In `@src/components/address_book_details.vue`:
- Around line 444-453: The discard confirmation dialog in
address_book_details.vue is using the title translation for both the heading and
body, so the warning text is missing. Update the dialog configuration in the
discard-edit flow to keep the title on dialog.discardEdit.title but point the
message field to the discard message translation key used for the body, leaving
the ok/cancel labels unchanged.
In `@src/components/bns/bns_input.vue`:
- Around line 203-205: The year label logic in bns_input.vue still hardcodes the
plural string in the content handling branch, so localized output can fall back
to English. Update the content formatting logic near the existing content
ternary to use translation keys or pluralization through this.$t for both
singular and plural forms, and keep the same behavior in the surrounding
formatter so all year text is fully i18n-driven.
In `@src/components/bns/bns_record_list.vue`:
- Around line 199-202: The renew button icon in bns_record_list.vue is exposing
the literal bnsRenew token through its alt text. Update the image used alongside
the $t("buttons.renew") label so it is decorative with an empty alt, or use the
same localized renew label for the alt text, and keep the change in the template
where the renew button/icon is rendered.
In `@src/components/bns/bns_update_input_form.vue`:
- Around line 55-57: The selector in bns_update_input_form.vue is only partially
localized, leaving the BChat, Belnet, ETH labels and nearby placeholder fallback
text hardcoded in English. Update the same template block to use the existing
i18n pattern via $t for every visible label and fallback string, including the
sibling option labels and any placeholder text, so the whole selector renders
consistently in all locales.
In `@src/css/app.styl`:
- Around line 934-940: The dropdown hover styles are targeting the wrong class
name, so they do not apply to the rendered component. Update the selector in
app.styl or the class binding in currencyDropDown.vue so the stylesheet and the
template use the same class name, and make sure the .dropDown rule matches the
actual dropdown element rendered by the component.
- Around line 1282-1292: The hover selector set in app.styl is incomplete for
the new button variants. Update the shared hover rules around the
send/transaction/contact/receive/addressbook buttons so .addressbook-btn:hover
is included alongside the other base hover selectors, and ensure the
active-state hover selector in the same block includes .active >
.transaction-send-btn:hover and uses .active > .addressbook-btn:hover
consistently. Keep the changes localized to the existing button selector groups
so the hover behavior matches across all right-pane actions.
In `@src/i18n/de.js`:
- Around line 69-71: The shared Open action label in the German i18n dictionary
is using the adjective form instead of the imperative verb form. Update the open
entry in the translation object so generic dialogs display the correct action
text, and keep the change localized to the relevant key in the i18n dictionary.
- Around line 187-191: The German locale still contains untranslated
mixed-language strings in the `de` catalog, including `sweepAllWarning.message`,
`invalidNameFormat`, `masterNodeStartStakingDescription1`, and
`noMasterNodesCurrentlyAvailable`. Update these entries in `src/i18n/de.js` so
they are fully localized to German, matching the surrounding keys and preserving
the same meaning for the dialogs and validation text.
In `@src/i18n/en-us.js`:
- Around line 248-257: The `decryptRecord` label in `en-us.js` was changed to
the add-record text, which breaks existing decrypt-flow callers. Update the
`fieldLabels.decryptRecord` entry to keep the decrypt-specific label, and use
the existing `strings.bns.addRecord` key only for the add-record action; make
the same correction anywhere the `decryptRecord` mapping was updated elsewhere
in this file so `decryptRecord` and `addRecord` stay distinct.
In `@src/i18n/es.js`:
- Around line 220-223: The Spanish renew confirmation label in confirmRenew has
a typo: the ok text currently shows “RENVAR” and should be corrected to
“RENOVAR”. Update the translation entry in the es.js i18n object so the dialog
button displays the proper Spanish verb.
- Around line 425-429: The Spanish name-validation strings are inconsistent:
invalidNameFormat and invalidNameHypenNotAllowed currently describe conflicting
allowed characters. Update the copy in es.js so both messages use the same
naming rules, and make sure invalidNameHypenNotAllowed matches the actual
validation logic around edge characters and any underscore/hyphen handling.
In `@src/i18n/fr.js`:
- Around line 267-270: The field label for fieldLabels.bnsType in the French
locale is mapped to the wrong concept, so update the translation in fr.js to
reflect the BNS record type rather than the backup-owner label. Use the existing
fieldLabels object entry for bnsType and replace the current text with the
correct French wording for BNS type, keeping the surrounding locale keys like
localDaemonIP and belnetFullAddress unchanged.
- Around line 229-239: The French unlock-master-node dialog still has a Spanish
action label in the unlockMasterNode object. Update the ok value in fr.js to use
the correct French text so the unlockMasterNode translation is fully localized,
and verify the related unlockMasterNodeWarning entry remains consistent with the
surrounding French strings.
In `@src/i18n/pt-br.js`:
- Around line 563-566: The new pt-BR strings in the i18n mapping still use
English copy, so translate the `encryptedBelnetValue`, `encryptedWalletValue`,
and `encryptedEthAddrValue` entries to Brazilian Portuguese to match
`encryptedBchatValue` and keep the BNS flow fully localized.
- Line 12: The pt-BR action labels in the buttons translation object are
incorrect and should preserve the intended UI actions. Update the entries for
buttons.clear, buttons.decrypt, and buttons.sign in the pt-br locale module so
they use proper Portuguese action labels instead of meanings like “Claro”,
“DESCRIPTO”, and “Sinal”. Locate the fixes in the buttons section of the locale
object where these keys are defined and align them with the rest of the
translations.
In `@src/i18n/ru.js`:
- Around line 218-220: The dialog.confirmRenew copy is using “update” wording
instead of renew wording, so the confirmation dialog shows the wrong action
text. Update the confirmRenew translation entries in the ru locale object to use
renew-specific phrasing, keeping the existing dialog.confirmRenew key and its
title/ok fields consistent with renewal terminology.
- Around line 365-366: Replace the leftover non-Russian strings in the ru locale
file so the Russian UI stays consistent. Update the affected translation
entries, including decryptedBNSRecord and the other noted strings around the
same locale object, to proper Russian text instead of Portuguese or English. Use
the existing keys in src/i18n/ru.js and keep the wording aligned with nearby
Russian messages in the same translation map.
- Line 126: The Russian translation title uses a mismatched interpolation token,
so update the title in the locale entry to use the same placeholder name as the
rest of the i18n strings. In the translation object in ru.js, keep the token
consistent with the surrounding keys by replacing the non-English placeholder in
the title with the standard one used by the dialog code, so the value passed as
type renders correctly.
In `@src/pages/wallet/swap.vue`:
- Around line 692-694: The placeholder interpolation in the recipient address
message uses the wrong key, so the locale token is not replaced correctly.
Update the call in the swap view where
this.$t('placeholders.enterRecipientAddress', ...) is used to pass the same
interpolation key expected by the locale message, matching the {coin}
placeholder instead of type. Make the change in the recipient address
placeholder logic near the sendAmounType usage so translated text renders the
currency name properly.
---
Nitpick comments:
In `@package.json`:
- Line 31: Remove the redundant npm https dependency from the project manifest
and lockfile. Update package.json to drop the https package entry, and make sure
package-lock.json no longer references that package. The relevant usage is
already covered by the Node core https module in
src-electron/main-process/modules/swap.js, so no code changes are needed there.
In `@src/components/menus/mainmenu.vue`:
- Around line 87-91: The version labels in mainmenu.vue are built by
concatenating English fragments with this.$t("strings.version"), which prevents
proper localization order. Update the Wallet and Daemon labels to use dedicated
full-sentence i18n keys with interpolation, and render them through the existing
template section so the noun, version term, and punctuation can be translated in
the correct order.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6619f110-65de-47aa-9916-b318abf129de
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (66)
.nvmrcbuild/entitlements.mac.plistpackage.jsonquasar.conf.jssrc-electron/main-process/electron-main.jssrc-electron/main-process/electron-preload.jssrc-electron/main-process/logging.jssrc-electron/main-process/modules/backend.jssrc-electron/main-process/modules/daemon.jssrc-electron/main-process/modules/swap.jssrc-electron/main-process/modules/wallet-rpc.jssrc/components/address_book_details.vuesrc/components/address_details.vuesrc/components/address_header.vuesrc/components/advanced/prove_transaction.vuesrc/components/advanced/sign_and_verify.vuesrc/components/bns/bns_input.vuesrc/components/bns/bns_input_form.vuesrc/components/bns/bns_record_list.vuesrc/components/bns/bns_records.vuesrc/components/bns/bns_renew.vuesrc/components/bns/bns_update.vuesrc/components/bns/bns_update_input_form.vuesrc/components/footer.vuesrc/components/icons/copy_icon.vuesrc/components/master_node/master_node_details.vuesrc/components/master_node/master_node_list.vuesrc/components/master_node/master_node_staking.vuesrc/components/master_node/master_node_unlock.vuesrc/components/menus/mainmenu.vuesrc/components/menus/wallet_settings.vuesrc/components/settings.vuesrc/components/settings_general.vuesrc/components/tx_details.vuesrc/components/tx_list.vuesrc/components/wallet_details.vuesrc/css/app.stylsrc/gateway/SCEE-Node.jssrc/gateway/gateway.jssrc/i18n/de.jssrc/i18n/en-us.jssrc/i18n/es.jssrc/i18n/fr.jssrc/i18n/pt-br.jssrc/i18n/ru.jssrc/index.template.htmlsrc/layouts/wallet/main.vuesrc/layouts/wallet/rightPane.vuesrc/pages/init/welcome.vuesrc/pages/wallet-select/create.vuesrc/pages/wallet-select/created.vuesrc/pages/wallet-select/import.vuesrc/pages/wallet-select/index.vuesrc/pages/wallet-select/restore.vuesrc/pages/wallet/addressbook.vuesrc/pages/wallet/currencyDropDown.vuesrc/pages/wallet/receive.vuesrc/pages/wallet/send.vuesrc/pages/wallet/swap.vuesrc/pages/wallet/swapTxnCompeleted.vuesrc/pages/wallet/swapTxnDetails.vuesrc/pages/wallet/swapTxnHistory.vuesrc/pages/wallet/swapTxnSettlement.vuesrc/pages/wallet/swapWaitingTxnHistory.vuesrc/pages/wallet/txhistory.vuesrc/shims/electron-renderer.js
💤 Files with no reviewable changes (1)
- src/gateway/SCEE-Node.js
Comment on lines
+53
to
+58
| const rendererConnectSrc = [ | ||
| "'self'", | ||
| "ws://127.0.0.1:12313", | ||
| "https://api.beldex.dev", | ||
| "https://api.changelly.com" | ||
| ]; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove direct renderer access to Changelly if swap calls are pinned in the main process.
Line 57 still allows renderer connect-src to https://api.changelly.com, which bypasses the new main-process TLS-pinned/signed request path if renderer code is compromised. Route those calls through IPC/backend and keep Changelly out of renderer CSP unless a remaining renderer call is intentional.
🔒 Proposed CSP tightening
const rendererConnectSrc = [
"'self'",
"ws://127.0.0.1:12313",
- "https://api.beldex.dev",
- "https://api.changelly.com"
+ "https://api.beldex.dev"
];📝 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.
Suggested change
| const rendererConnectSrc = [ | |
| "'self'", | |
| "ws://127.0.0.1:12313", | |
| "https://api.beldex.dev", | |
| "https://api.changelly.com" | |
| ]; | |
| const rendererConnectSrc = [ | |
| "'self'", | |
| "ws://127.0.0.1:12313", | |
| "https://api.beldex.dev" | |
| ]; |
🤖 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 `@src-electron/main-process/electron-main.js` around lines 53 - 58, The
renderer CSP in electron-main.js still whitelists https://api.changelly.com,
which bypasses the new main-process request path. Remove Changelly from renderer
connect-src in rendererConnectSrc unless there is a deliberate remaining
renderer call, and keep those swap requests routed through IPC/backend via the
main-process path instead.
Comment on lines
+231
to
+240
| powerMonitor.on("suspend", () => { | ||
| mainWindow.webContents.send("appSuspend"); | ||
| }); | ||
|
|
||
| powerMonitor.on("resume", () => { | ||
| let config = { | ||
| port: 12313, | ||
| token: startingToken | ||
| }; | ||
| mainWindow.webContents.send("appResumed", config); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard power events before sending resume IPC.
These handlers can dereference mainWindow.webContents before the window/backend is ready or after it is destroyed, and appResumed can send a null startingToken. The gateway consumes data.token, so skip resume until both the window and token exist.
🛡️ Proposed guard
powerMonitor.on("suspend", () => {
+ if (!mainWindow || mainWindow.isDestroyed()) {
+ return;
+ }
mainWindow.webContents.send("appSuspend");
});
powerMonitor.on("resume", () => {
+ if (!mainWindow || mainWindow.isDestroyed() || !startingToken) {
+ return;
+ }
let config = {
port: 12313,
token: startingToken📝 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.
Suggested change
| powerMonitor.on("suspend", () => { | |
| mainWindow.webContents.send("appSuspend"); | |
| }); | |
| powerMonitor.on("resume", () => { | |
| let config = { | |
| port: 12313, | |
| token: startingToken | |
| }; | |
| mainWindow.webContents.send("appResumed", config); | |
| powerMonitor.on("suspend", () => { | |
| if (!mainWindow || mainWindow.isDestroyed()) { | |
| return; | |
| } | |
| mainWindow.webContents.send("appSuspend"); | |
| }); | |
| powerMonitor.on("resume", () => { | |
| if (!mainWindow || mainWindow.isDestroyed() || !startingToken) { | |
| return; | |
| } | |
| let config = { | |
| port: 12313, | |
| token: startingToken | |
| }; | |
| mainWindow.webContents.send("appResumed", config); |
🤖 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 `@src-electron/main-process/electron-main.js` around lines 231 - 240, The
powerMonitor handlers in electron-main.js should guard against sending IPC
before the app is ready. Update the suspend/resume listeners so they only call
mainWindow.webContents.send when mainWindow and mainWindow.webContents exist and
the window is not destroyed, and make the resume path skip or defer appResumed
unless startingToken is present. Keep the fix localized to the
powerMonitor.on("suspend") and powerMonitor.on("resume") flow so the gateway
always receives a valid config.token.
Comment on lines
+10
to
+27
| function cleanArgsForIPC(args) { | ||
| const redactKeyPattern = /(password|seed|mnemonic|secret|spend[_-]?key|view[_-]?key|private[_-]?key|auth|token)/i; | ||
| const redactValue = item => { | ||
| if (Array.isArray(item)) { | ||
| return item.map(redactValue); | ||
| } | ||
|
|
||
| if (item && typeof item === "object") { | ||
| return Object.keys(item).reduce((result, key) => { | ||
| result[key] = redactKeyPattern.test(key) | ||
| ? "[REDACTED]" | ||
| : redactValue(item[key]); | ||
| return result; | ||
| }, {}); | ||
| } | ||
|
|
||
| return item; | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact nested sensitive strings before logging.
redactValue() only redacts sensitive keys; nested values like { message: "seed phrase: ..." } are preserved and then logged on Line 33. Apply the same string redaction inside redactValue().
🛡️ Proposed fix
const redactValue = item => {
+ if (typeof item === "string") {
+ return redactKeyPattern.test(item) ? "[REDACTED]" : item;
+ }
+
if (Array.isArray(item)) {
return item.map(redactValue);
}📝 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.
Suggested change
| function cleanArgsForIPC(args) { | |
| const redactKeyPattern = /(password|seed|mnemonic|secret|spend[_-]?key|view[_-]?key|private[_-]?key|auth|token)/i; | |
| const redactValue = item => { | |
| if (Array.isArray(item)) { | |
| return item.map(redactValue); | |
| } | |
| if (item && typeof item === "object") { | |
| return Object.keys(item).reduce((result, key) => { | |
| result[key] = redactKeyPattern.test(key) | |
| ? "[REDACTED]" | |
| : redactValue(item[key]); | |
| return result; | |
| }, {}); | |
| } | |
| return item; | |
| }; | |
| function cleanArgsForIPC(args) { | |
| const redactKeyPattern = /(password|seed|mnemonic|secret|spend[_-]?key|view[_-]?key|private[_-]?key|auth|token)/i; | |
| const redactValue = item => { | |
| if (typeof item === "string") { | |
| return redactKeyPattern.test(item) ? "[REDACTED]" : item; | |
| } | |
| if (Array.isArray(item)) { | |
| return item.map(redactValue); | |
| } | |
| if (item && typeof item === "object") { | |
| return Object.keys(item).reduce((result, key) => { | |
| result[key] = redactKeyPattern.test(key) | |
| ? "[REDACTED]" | |
| : redactValue(item[key]); | |
| return result; | |
| }, {}); | |
| } | |
| return item; | |
| }; |
🤖 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 `@src-electron/main-process/electron-preload.js` around lines 10 - 27, The
cleanArgsForIPC redactValue helper only redacts based on key names, so sensitive
substrings inside nested string values can still reach the logger. Update
redactValue in electron-preload.js to apply the same redaction rules to string
leaf values as well as to objects and arrays, and keep using cleanArgsForIPC
where the sanitized args are logged. Use the existing redactKeyPattern and
redactValue symbols so nested values like message fields are redacted before
logging.
Comment on lines
+216
to
+218
| shell: { | ||
| openExternal: url => shell.openExternal(url) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate external URLs before calling shell.openExternal.
This bypasses the main-process isSafeExternalUrl() guard and lets any renderer/XSS path launch arbitrary schemes. Restrict to the same allowed protocols before forwarding.
🛡️ Proposed fix
shell: {
- openExternal: url => shell.openExternal(url)
+ openExternal: url => {
+ try {
+ const parsedUrl = new URL(url);
+ if (parsedUrl.protocol === "https:" || parsedUrl.protocol === "http:") {
+ return shell.openExternal(url);
+ }
+ } catch (error) {
+ // Ignore invalid URLs.
+ }
+ return Promise.resolve();
+ }
}📝 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.
Suggested change
| shell: { | |
| openExternal: url => shell.openExternal(url) | |
| } | |
| shell: { | |
| openExternal: url => { | |
| try { | |
| const parsedUrl = new URL(url); | |
| if (parsedUrl.protocol === "https:" || parsedUrl.protocol === "http:") { | |
| return shell.openExternal(url); | |
| } | |
| } catch (error) { | |
| // Ignore invalid URLs. | |
| } | |
| return Promise.resolve(); | |
| } | |
| } |
🤖 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 `@src-electron/main-process/electron-preload.js` around lines 216 - 218, The
shell.openExternal bridge in electron-preload.js forwards any URL directly from
the renderer, bypassing the main-process isSafeExternalUrl() check. Update the
openExternal handler to validate the incoming url against the same allowed
external protocols before calling shell.openExternal, and reject or no-op for
anything else. Keep the fix localized to the shell.openExternal path so the
preload only exposes the already-approved navigation behavior.
Comment on lines
+30
to
+46
| const redactKeyPattern = /(password|seed|mnemonic|secret|spend[_-]?key|view[_-]?key|private[_-]?key|auth|token)/i; | ||
| const redactValue = item => { | ||
| if (Array.isArray(item)) { | ||
| return item.map(redactValue); | ||
| } | ||
|
|
||
| if (item && typeof item === "object") { | ||
| return Object.keys(item).reduce((result, key) => { | ||
| result[key] = redactKeyPattern.test(key) | ||
| ? "[REDACTED]" | ||
| : redactValue(item[key]); | ||
| return result; | ||
| }, {}); | ||
| } | ||
|
|
||
| return item; | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact nested sensitive string values here too.
This path still logs nested strings containing sensitive labels when the containing key is benign, e.g. { message: "password=..." }.
🛡️ Proposed fix
const redactValue = item => {
+ if (typeof item === "string") {
+ return redactKeyPattern.test(item) ? "[REDACTED]" : item;
+ }
+
if (Array.isArray(item)) {
return item.map(redactValue);
}📝 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.
Suggested change
| const redactKeyPattern = /(password|seed|mnemonic|secret|spend[_-]?key|view[_-]?key|private[_-]?key|auth|token)/i; | |
| const redactValue = item => { | |
| if (Array.isArray(item)) { | |
| return item.map(redactValue); | |
| } | |
| if (item && typeof item === "object") { | |
| return Object.keys(item).reduce((result, key) => { | |
| result[key] = redactKeyPattern.test(key) | |
| ? "[REDACTED]" | |
| : redactValue(item[key]); | |
| return result; | |
| }, {}); | |
| } | |
| return item; | |
| }; | |
| const redactKeyPattern = /(password|seed|mnemonic|secret|spend[_-]?key|view[_-]?key|private[_-]?key|auth|token)/i; | |
| const redactValue = item => { | |
| if (typeof item === "string") { | |
| return redactKeyPattern.test(item) ? "[REDACTED]" : item; | |
| } | |
| if (Array.isArray(item)) { | |
| return item.map(redactValue); | |
| } | |
| if (item && typeof item === "object") { | |
| return Object.keys(item).reduce((result, key) => { | |
| result[key] = redactKeyPattern.test(key) | |
| ? "[REDACTED]" | |
| : redactValue(item[key]); | |
| return result; | |
| }, {}); | |
| } | |
| return item; | |
| }; |
🤖 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 `@src-electron/main-process/logging.js` around lines 30 - 46, The redaction
logic in redactValue only hides sensitive data when the object key matches, so
nested string values like message fields can still leak secrets. Update
redactValue in logging.js to also inspect string values recursively (and redact
or mask them when they contain sensitive labels/patterns, not just when the key
matches), while preserving the existing behavior for arrays and objects. Keep
the fix localized to redactKeyPattern and redactValue so all logged payloads are
sanitized consistently.
Comment on lines
+532
to
+539
| derivePasswordHashSync(password) { | ||
| return crypto.pbkdf2Sync( | ||
| password, | ||
| this.auth[2], | ||
| PASSWORD_HASH_PBKDF2_ITERATIONS, | ||
| PASSWORD_HASH_KEY_LENGTH, | ||
| PASSWORD_HASH_DIGEST | ||
| ); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file regions and the async helper/call sites.
python3 - <<'PY'
from pathlib import Path
path = Path('src-electron/main-process/modules/wallet-rpc.js')
lines = path.read_text().splitlines()
targets = [(500, 560), (580, 630), (650, 700), (720, 780), (790, 850), (870, 930), (980, 1045), (2890, 2945)]
for a,b in targets:
print(f"\n--- {path}:{a}-{b} ---")
for i in range(a, min(b, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
PY
# Find async/sync password hash helpers and references.
rg -n "derivePasswordHash(Sync)?\(" src-electron/main-process/modules/wallet-rpc.jsRepository: Beldex-Coin/beldex-electron-gui-wallet
Length of output: 18756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the PBKDF2 constants and run a small local benchmark for the sync path.
python3 - <<'PY'
from pathlib import Path
import re
text = Path('src-electron/main-process/modules/wallet-rpc.js').read_text()
for name in ['PASSWORD_HASH_PBKDF2_ITERATIONS', 'PASSWORD_HASH_KEY_LENGTH', 'PASSWORD_HASH_DIGEST']:
m = re.search(rf'const\s+{name}\s*=\s*([^;\n]+)', text)
print(name, "=", m.group(1).strip() if m else "NOT_FOUND")
PY
node - <<'JS'
const crypto = require('crypto');
const password = 'test-password';
const salt = 'test-salt';
const iterations = 600000;
const keylen = 32;
const digest = 'sha256';
const samples = 5;
let times = [];
for (let i = 0; i < samples; i++) {
const t0 = process.hrtime.bigint();
crypto.pbkdf2Sync(password, salt, iterations, keylen, digest);
const ms = Number(process.hrtime.bigint() - t0) / 1e6;
times.push(ms);
console.log(`sample ${i + 1}: ${ms.toFixed(1)} ms`);
}
times.sort((a, b) => a - b);
const avg = times.reduce((a, b) => a + b, 0) / times.length;
console.log(`min=${times[0].toFixed(1)} ms median=${times[Math.floor(times.length/2)].toFixed(1)} ms avg=${avg.toFixed(1)} ms max=${times[times.length-1].toFixed(1)} ms`);
JSRepository: Beldex-Coin/beldex-electron-gui-wallet
Length of output: 429
Avoid synchronous PBKDF2 on the Electron main process.
pbkdf2Sync blocks this flow for ~300ms at 600k iterations, so wallet open/create/restore/import/password-change can briefly freeze the UI. Use derivePasswordHash here too.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import child_process from "child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🤖 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 `@src-electron/main-process/modules/wallet-rpc.js` around lines 532 - 539, The
synchronous PBKDF2 call in derivePasswordHashSync is blocking the Electron main
process and can freeze wallet flows; update the wallet-rpc.js password hashing
path to use the async derivePasswordHash helper instead of crypto.pbkdf2Sync,
and adjust the call sites that open/create/restore/import/change passwords so
they await the asynchronous result.
Comment on lines
+2061
to
+2063
| if (backupOwner) { | ||
| params.backupOwner = backupOwner; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the RPC’s backup_owner parameter name.
purchaseBNS sends backup_owner, but updateBNSMapping sends backupOwner; the update RPC likely ignores the backup owner change.
Proposed fix
if (backupOwner) {
- params.backupOwner = backupOwner;
+ params.backup_owner = backupOwner;
}📝 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.
Suggested change
| if (backupOwner) { | |
| params.backupOwner = backupOwner; | |
| } | |
| if (backupOwner) { | |
| params.backup_owner = backupOwner; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import child_process from "child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🤖 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 `@src-electron/main-process/modules/wallet-rpc.js` around lines 2061 - 2063,
The update flow in updateBNSMapping is using the wrong RPC field name for the
backup owner, so the change is likely ignored. Update the params assignment in
updateBNSMapping to use the RPC’s backup_owner key, matching the naming already
used by purchaseBNS, and keep the rest of the request shape unchanged.
| const daemonType = this.config_daemon.type; | ||
| const isSyncing = | ||
| this.daemon.info.height_without_bootstrap < this.target_height; | ||
| const isSyncing = this.daemon.info.height < this.target_height; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the same height source for local_remote syncing status.
Line 101 makes isSyncing impossible for local_remote: target_height() returns daemon.info.height for that mode, so this becomes height < height and the footer will report ready while the local daemon is still catching up.
Suggested fix
- const isSyncing = this.daemon.info.height < this.target_height;
+ const isSyncing = this.daemon_height < this.target_height;📝 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.
Suggested change
| const isSyncing = this.daemon.info.height < this.target_height; | |
| const isSyncing = this.daemon_height < this.target_height; |
🤖 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 `@src/components/footer.vue` at line 101, The syncing check in footer.vue uses
`this.target_height` for `isSyncing`, which breaks `local_remote` because
`target_height()` resolves to `daemon.info.height` and makes the comparison
always false. Update the `isSyncing` logic in the footer component to use the
same height source consistently for `local_remote`, following the existing
`target_height()`/daemon height handling in the component so the footer stays in
syncing state until the local daemon actually catches up.
Comment on lines
+8
to
+13
| <% if (ctx.prod) { %> | ||
| <meta | ||
| http-equiv="Content-Security-Policy" | ||
| content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' ws://127.0.0.1:12313 https://api.beldex.dev https://api.changelly.com; object-src 'none'; base-uri 'none'; form-action 'self';" | ||
| /> | ||
| <% } %> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Mirror the tightened renderer CSP here as well.
If Changelly traffic is meant to go through the main-process pinned/signed swap path, remove https://api.changelly.com from this production meta CSP too. This is the same policy gap as in electron-main.js.
🔒 Proposed CSP tightening
- content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' ws://127.0.0.1:12313 https://api.beldex.dev https://api.changelly.com; object-src 'none'; base-uri 'none'; form-action 'self';"
+ content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' ws://127.0.0.1:12313 https://api.beldex.dev; object-src 'none'; base-uri 'none'; form-action 'self';"📝 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.
Suggested change
| <% if (ctx.prod) { %> | |
| <meta | |
| http-equiv="Content-Security-Policy" | |
| content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' ws://127.0.0.1:12313 https://api.beldex.dev https://api.changelly.com; object-src 'none'; base-uri 'none'; form-action 'self';" | |
| /> | |
| <% } %> | |
| <% if (ctx.prod) { %> | |
| <meta | |
| http-equiv="Content-Security-Policy" | |
| content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' ws://127.0.0.1:12313 https://api.beldex.dev; object-src 'none'; base-uri 'none'; form-action 'self';" | |
| /> | |
| <% } %> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 8-8: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 8-8: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 13-13: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 13-13: Special characters must be escaped : [ > ].
(spec-char-escape)
🤖 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 `@src/index.template.html` around lines 8 - 13, The production
Content-Security-Policy in the index.template.html meta tag still allows
https://api.changelly.com, which leaves the renderer CSP looser than the
tightened main-process policy. Update the CSP string in the template so it
matches the renderer’s pinned/signed swap path policy by removing the Changelly
endpoint from connect-src, and keep the rest of the meta CSP aligned with the
symbols/condition around ctx.prod in the template.
Comment on lines
+19
to
29
| <div class="sendAndReceiveBtn"> | ||
| <div | ||
| :class="[this.routes === 'send' ? 'active' : '']" | ||
| style="margin-right:8px" | ||
| > | ||
| <!-- width="16" | ||
| height="20" --> | ||
| <svg | ||
| class="icon" | ||
| viewBox="0 0 16 20" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| d="M0.289064 6.61106C0.0558891 6.88987 0.0586794 7.34525 0.295297 7.62817C0.334142 7.67441 0.377977 7.71422 0.425604 7.74651C0.426714 7.74719 0.427825 7.74786 0.428939 7.74853L3.71653 10.7093L3.3068 13.2484L13.0299 3.67152L5.02053 15.2975L7.144 14.8076L9.62101 18.7396C9.64809 18.7976 9.68168 18.8511 9.72083 18.8984C9.95745 19.1813 10.3383 19.1846 10.5715 18.9058C10.6273 18.8383 10.6711 18.758 10.7004 18.6696L10.7021 18.6696L10.711 18.6369L10.7159 18.621C10.717 18.6164 10.7181 18.6118 10.7192 18.6072L15.2786 1.92726L15.2777 1.92427C15.3658 1.65816 15.3119 1.3526 15.1409 1.14737C14.9689 0.94202 14.7125 0.877606 14.4895 0.983742L14.4886 0.982741L14.4878 0.983727L0.543811 6.43253C0.524437 6.43882 0.505354 6.44628 0.486633 6.45487C0.412525 6.49049 0.345349 6.5436 0.289064 6.61106Z" | ||
| fill="white" | ||
| /> | ||
| </svg> | ||
| <!-- <span class="btn-txt">{{ $t("buttons.send") }}</span> --> | ||
| <span class="btn-txt">Send</span> | ||
| </q-btn> | ||
| </div> | ||
| <!-- </router-link> --> | ||
| <!-- <router-link to="/wallet/receive"> --> | ||
| <div :class="[this.routes === 'receive' ? 'active' : '']"> | ||
| <q-btn class="large-btn send-btn" @click="router('receive')"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 96 960 960" | ||
| class="icon" | ||
| <!-- <q-btn class="large-btn send-btn" size="md" @click="router('send')"> --> | ||
| <div | ||
| class="large-btn transaction-send-btn" | ||
| :disable="view_only" | ||
| @click="router('send')" | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '15,60p' src/layouts/wallet/rightPane.vue
rg -n --type=vue -C2 '\bview_only\b|set_router_path_rightpane|router_path_rightpane' srcRepository: Beldex-Coin/beldex-electron-gui-wallet
Length of output: 2638
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section of rightPane.vue
nl -ba src/layouts/wallet/rightPane.vue | sed -n '110,160p'
# Search for view-only and right-pane routing logic across the repo
rg -n --type=vue --type=js --type=ts --type=jsx --type=tsx '\bview_only\b|set_router_path_rightpane|router_path_rightpane|router\(\s*[\'"]send[\'"]\s*\)|router\(\s*[\'"]receive[\'"]\s*\)' src
# Look for downstream send/receive page guards
rg -n --type=vue --type=js --type=ts '\bview_only\b|send.*view|receive.*view|wallet/send|wallet/receive' srcRepository: Beldex-Coin/beldex-electron-gui-wallet
Length of output: 216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba src/layouts/wallet/rightPane.vue | sed -n '110,160p'
rg -n --type=vue --type=js --type=ts --type=jsx --type=tsx '\bview_only\b|set_router_path_rightpane|router_path_rightpane|router\(\s*[\'"]send[\'"]\s*\)|router\(\s*[\'"]receive[\'"]\s*\)' src
rg -n --type=vue --type=js --type=ts '\bview_only\b|send.*view|receive.*view|wallet/send|wallet/receive' srcRepository: Beldex-Coin/beldex-electron-gui-wallet
Length of output: 216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the router handler with a tool that exists here
sed -n '110,160p' src/layouts/wallet/rightPane.vue | cat -n
# Search for route/view-only handling in the wallet area
rg -n --type=vue --type=js --type=ts --type=jsx --type=tsx '\bview_only\b|set_router_path_rightpane|router_path_rightpane|router\(\s*[\'"]send[\'"]\s*\)|router\(\s*[\'"]receive[\'"]\s*\)' src
# Look for any downstream send/receive page guards
rg -n --type=vue --type=js --type=ts '\bview_only\b|wallet/send|wallet/receive|send.*view|receive.*view' srcRepository: Beldex-Coin/beldex-electron-gui-wallet
Length of output: 1433
Keep the send/receive controls as buttons. :disable="view_only" does nothing on a <div>, so view-only wallets can still open these panes, and the primary actions lose keyboard focus/activation.
🤖 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 `@src/layouts/wallet/rightPane.vue` around lines 19 - 29, The send/receive
controls in rightPane.vue are implemented as a div, so the disable state on the
transaction-send-btn is ineffective and the action is not keyboard-accessible.
Update the send/receive control markup in the sendAndReceiveBtn section to use
an actual button-based component or element, and wire the disabled state to
view_only so it cannot be activated in view-only wallets. Keep the existing
router('send') behavior and preserve the active-state class logic tied to
this.routes.
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
4 participants
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.
Summary by CodeRabbit