Skip to content

fix: add input validation for account creation #3490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abhishek-8081 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: add input validation for account creation #3490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3887f41
fix: add input validation for account creation
abhishek-8081 Feb 13, 2026
57c0861
Merge remote-tracking branch 'upstream/master' into fix/add-account-v…
pavanjoshi914 Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions src/extension/background-script/actions/accounts/__tests__/add.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,26 @@ import type { MessageAccountAdd } from "~/types";
import addAccount from "../add";

jest.mock("~/extension/background-script/state");
jest.mock("~/extension/background-script/connectors", () => ({
__esModule: true,
default: {
lnd: {},
nativelnd: {},
lndhub: {},
nativelndhub: {},
kollider: {},
lnbits: {},
lnc: {},
nativelnbits: {},
galoy: {},
eclair: {},
citadel: {},
nativecitadel: {},
alby: {},
nwc: {},
lawallet: {},
},
}));
jest.mock("uuid", () => {
return {
v4: jest.fn(() => "random-id-42"),
Expand Down Expand Up @@ -128,4 +148,55 @@ describe("add account to account-list", () => {

expect(spy).toHaveBeenCalledTimes(1);
});

test("returns error for invalid connector type", async () => {
const mockState = defaultMockState;
state.getState = jest.fn().mockReturnValue(mockState);

const invalidMessage: MessageAccountAdd = {
...message,
args: {
...message.args,
connector: "fakeconnector" as never,
},
};

expect(await addAccount(invalidMessage)).toStrictEqual({
error: "Invalid connector type",
});
});

test("returns error for missing config", async () => {
const mockState = defaultMockState;
state.getState = jest.fn().mockReturnValue(mockState);

const invalidMessage: MessageAccountAdd = {
...message,
args: {
...message.args,
config: "",
},
};

expect(await addAccount(invalidMessage)).toStrictEqual({
error: "Account config is required",
});
});

test("returns error for missing name", async () => {
const mockState = defaultMockState;
state.getState = jest.fn().mockReturnValue(mockState);

const invalidMessage: MessageAccountAdd = {
...message,
args: {
...message.args,
name: "",
},
};

expect(await addAccount(invalidMessage)).toStrictEqual({
error: "Account name is required",
});
});
});
27 changes: 25 additions & 2 deletions src/extension/background-script/actions/accounts/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,40 @@
import { v4 as uuidv4 } from "uuid";
import { encryptData } from "~/common/lib/crypto";
import { getUniqueAccountName } from "~/common/utils/validations";
import connectors from "~/extension/background-script/connectors";
import state from "~/extension/background-script/state";
import type { MessageAccountAdd } from "~/types";

const add = async (message: MessageAccountAdd) => {
const newAccount = message.args;

// Validate connector type
if (!newAccount.connector || !(newAccount.connector in connectors)) {
return { error: "Invalid connector type" };
}

// Validate config is present
if (
!newAccount.config ||
typeof newAccount.config !== "string" ||
newAccount.config.trim() === ""
) {
return { error: "Account config is required" };
}
Comment on lines +17 to +23

@coderabbitai coderabbitai Bot Feb 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1. Find the Account type definition
echo "=== Checking Account type definition ==="
find . -name "*.ts" -type f | xargs rg -l "type Account|interface Account" | head -5

Repository: getAlby/lightning-browser-extension

Length of output: 272

🏁 Script executed:

#!/bin/bash
# 2. Check the alby provider method signature
echo "=== Checking alby provider signature ==="
cat -n src/extension/providers/alby/index.ts | head -50

Repository: getAlby/lightning-browser-extension

Length of output: 1441

🏁 Script executed:

#!/bin/bash
# 3. Find all files that reference addAccount
echo "=== Files referencing addAccount ==="
rg -l "addAccount" --type=ts src/ | grep -v node_modules

Repository: getAlby/lightning-browser-extension

Length of output: 1241

🏁 Script executed:

#!/bin/bash
# 4. Check how the add action is called from connectors
echo "=== Tracing the call flow to add action ==="
rg -B5 -A5 "\.add\(" src/ --type=ts | head -100

Repository: getAlby/lightning-browser-extension

Length of output: 7954

🏁 Script executed:

#!/bin/bash
# Check the Account type definition in detail
cat -n src/types.ts | grep -A 10 "type Account\|interface Account"

Repository: getAlby/lightning-browser-extension

Length of output: 932

🏁 Script executed:

#!/bin/bash
# Check the add action file more thoroughly
cat -n src/extension/background-script/actions/accounts/add.ts

Repository: getAlby/lightning-browser-extension

Length of output: 2323

🏁 Script executed:

#!/bin/bash
# Check how addAccount is called in the router
cat -n src/extension/background-script/router.ts | grep -B 10 -A 10 "addAccount"

Repository: getAlby/lightning-browser-extension

Length of output: 1760

🏁 Script executed:

#!/bin/bash
# Check the MessageAccountAdd type definition
rg -B 5 -A 10 "MessageAccountAdd" src/types.ts

Repository: getAlby/lightning-browser-extension

Length of output: 556

🏁 Script executed:

#!/bin/bash
# Check the promptAdd action to understand the flow
cat -n src/extension/background-script/actions/accounts/promptAdd.ts

Repository: getAlby/lightning-browser-extension

Length of output: 1121

🏁 Script executed:

#!/bin/bash
# Check how connectors actually send data to addAccount
rg -B 10 -A 5 "promptAdd\|addAccount" src/app/screens/connectors/ConnectAlby/index.tsx

Repository: getAlby/lightning-browser-extension

Length of output: 61

🏁 Script executed:

#!/bin/bash
# Check the ConfirmAddAccount screen to see how it transforms config
cat -n src/app/screens/ConfirmAddAccount/index.tsx

Repository: getAlby/lightning-browser-extension

Length of output: 3899

🏁 Script executed:

#!/bin/bash
# Check what happens after confirmation
rg -B 5 -A 10 "confirmAddAccount\|addAccount" src/app/screens/ConfirmAddAccount/index.tsx

Repository: getAlby/lightning-browser-extension

Length of output: 61

🏁 Script executed:

#!/bin/bash
# Check the message flow in the background router more carefully
cat -n src/extension/background-script/router.ts | head -110

Repository: getAlby/lightning-browser-extension

Length of output: 4651

🏁 Script executed:

#!/bin/bash
# Check how msg.request works - might do serialization
rg -B 5 -A 15 "request.*function\|export.*request\|const request" src/common/lib/msg.ts | head -60

Repository: getAlby/lightning-browser-extension

Length of output: 61

🏁 Script executed:

#!/bin/bash
# Look at the actual test to see if config is string or object
cat -n src/extension/background-script/actions/accounts/__tests__/add.test.ts | head -100

Repository: getAlby/lightning-browser-extension

Length of output: 3051

🏁 Script executed:

#!/bin/bash
# Check if there's any JSON serialization happening in the flow
rg "JSON.stringify" src/app/screens/ConfirmAddAccount/ src/common/lib/msg.ts src/extension/background-script/

Repository: getAlby/lightning-browser-extension

Length of output: 2513

🏁 Script executed:

#!/bin/bash
# Check how msg.request implementation works to see if it serializes
find . -name "msg.ts" -o -name "msg.js" | grep -E "common/lib" | head -3

Repository: getAlby/lightning-browser-extension

Length of output: 102

🏁 Script executed:

#!/bin/bash
# Read the msg.ts implementation
cat -n src/common/lib/msg.ts

Repository: getAlby/lightning-browser-extension

Length of output: 1384

🏁 Script executed:

#!/bin/bash
# Check if there are any other usages showing config type expectations
rg -B 3 -A 3 "config.*object\|config.*Record" src/app/screens/connectors/ --type=ts | head -50

Repository: getAlby/lightning-browser-extension

Length of output: 61

🏁 Script executed:

#!/bin/bash
# Check one specific connector to see how config is handled
cat -n src/app/screens/connectors/ConnectLnd/index.tsx | head -150

Repository: getAlby/lightning-browser-extension

Length of output: 5524

🏁 Script executed:

#!/bin/bash
# Search for where config might be stringified in connector screens
rg "JSON.stringify.*config\|config.*JSON.stringify" src/app/screens/connectors/ --type=tsx

Repository: getAlby/lightning-browser-extension

Length of output: 106

🏁 Script executed:

#!/bin/bash
# Check how connectors are setting config before calling promptAdd/addAccount
rg -B 10 "msg.request.*addAccount\|promptAdd" src/app/screens/connectors/ --type=tsx | head -80

Repository: getAlby/lightning-browser-extension

Length of output: 106

🏁 Script executed:

#!/bin/bash
# Look for more connector examples to confirm the pattern
rg -B 5 -A 5 "config.*\{" src/app/screens/connectors/ConnectNWC/index.tsx | head -40

Repository: getAlby/lightning-browser-extension

Length of output: 318

🏁 Script executed:

#!/bin/bash
# Check if there's any middleware that transforms the message before it reaches add.ts
cat -n src/extension/background-script/index.ts

Repository: getAlby/lightning-browser-extension

Length of output: 8580

🏁 Script executed:

#!/bin/bash
# Search for message listeners that might transform config to string
rg "runtime.onMessage\|sendMessage" src/extension/background-script/ --type=ts -B 3 -A 10 | head -100

Repository: getAlby/lightning-browser-extension

Length of output: 61

🏁 Script executed:

#!/bin/bash
# Double-check by looking at one more connector to confirm the pattern
rg -B 3 -A 8 "msg.request.*addAccount" src/app/screens/connectors/ConnectLnbits/ -A 10 | head -30

Repository: getAlby/lightning-browser-extension

Length of output: 1225

Fix type mismatch: config arrives as an object but validation expects a string.

In the actual connector UI flow, config is created as an object (e.g., { url, macaroon } in ConnectLnd), passed through promptAdd (which validates typeof config !== "object" at line 17), and forwarded unchanged by ConfirmAddAccount. However, the validation at lines 17-23 in add.ts requires config to be a string and will reject the object. Since browser.runtime.sendMessage() preserves object types with no serialization in between, the validation will fail when called from the UI.

Either serialize config to a JSON string before this point, or update the validation to accept objects matching the Record<string, unknown> signature defined in AlbyProvider.

🤖 Prompt for AI Agents 
In `@src/extension/background-script/actions/accounts/add.ts` around lines 17 -
23, The validation for newAccount.config in add.ts incorrectly requires a
string; update it to accept either a string or an object matching Record<string,
unknown> (per AlbyProvider) so objects like { url, macaroon } passed from
promptAdd/ConfirmAddAccount are allowed; specifically, modify the check around
newAccount.config to allow typeof === "object" (and non-null) or a non-empty
string, and ensure subsequent code that uses newAccount.config handles both
serialized strings and object shapes (or normalize by JSON.stringify only if you
choose to serialize upstream).


// Validate name is present
if (
!newAccount.name ||
typeof newAccount.name !== "string" ||
newAccount.name.trim() === ""
) {
return { error: "Account name is required" };
}

const accounts = state.getState().accounts;
const name = getUniqueAccountName(newAccount.name, accounts);
const tmpAccounts = { ...accounts };

// TODO: add validations
// TODO: make sure a password is set
const password = await state.getState().password();
if (!password) return { error: "Password is missing" };

Expand Down