Skip to content

fix: protect Stellar wallet lookups #48

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
Banx17 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: protect Stellar wallet lookups #48

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
5c8abc0
fix: protect Stellar wallet lookups
Banx17 Jul 21, 2026
0459f21
stellar: validate signed XDR contents before submit; store expectedHa…
Banx17 Jul 21, 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
1 change: 1 addition & 0 deletions jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export default {
testEnvironment: "node",
transform: {},
setupFiles: ["<rootDir>/test/jest.setup.js"],
};
9 changes: 9 additions & 0 deletions src/config/validateEnv.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ export const validateEnv = () => {
process.env.ACCESS_TOKEN_TTL = process.env.ACCESS_TOKEN_TTL || "15m";
process.env.REFRESH_TOKEN_TTL = process.env.REFRESH_TOKEN_TTL || "30d";

// During tests, relax strict env validation and provide sane defaults
if (process.env.NODE_ENV === "test") {
process.env.JWT_SECRET = process.env.JWT_SECRET || "test-secret";
process.env.MONGO_URI = process.env.MONGO_URI || "mongodb://127.0.0.1:27017/deenbridge_test";
process.env.PORT = process.env.PORT || "3000";
logger.info("✅ Skipping strict env validation in test mode");
return;
Comment on lines +34 to +40

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Do not embed test configuration defaults in source.

The fallback JWT secret and Mongo connection string mask missing test/CI configuration and violate the environment-only configuration rule. This shares the same root cause as test/jest.setup.js Line 3 and is consolidated below.

As per path instructions, “all configuration belongs in environment variables documented in .env.example.”

🤖 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/config/validateEnv.js` around lines 34 - 40, Remove the hardcoded
JWT_SECRET and MONGO_URI fallbacks from the test-mode branch in the environment
validation flow, including the assignments in validateEnv. Keep configuration
sourced exclusively from environment variables documented in .env.example, while
preserving only the test-mode behavior and any non-sensitive default that is
explicitly allowed.

Source: Path instructions

}

const missing = [];

requiredEnvVars.forEach((envVar) => {
Expand Down
31 changes: 29 additions & 2 deletions src/controllers/stellar/donationController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ export const initializeDonation = async (req, res) => {
amount: amount.toString(),
network: NETWORK,
status: "pending",
stellarTxHash: paymentTx.hash, // Temporary hash, will be replaced with actual
expectedHash: paymentTx.hash,
memo: DONATION_MEMO,
});

await donation.save({ session });
Expand Down Expand Up @@ -158,7 +159,33 @@ export const submitDonation = async (req, res) => {
});
}

// Update status to submitted
// Build expected payment and validate signed XDR BEFORE submit
const expectedPayments = [
{
destination: donation.creatorWallet,
amount: donation.amount,
},
];

try {
validateSignedPaymentXdr(signedXdr, expectedPayments, donation.memo, donation.buyerWallet, true);
} catch (validationError) {
donation.status = "failed";
donation.failureReason = `validation_failed: ${validationError.message}`;
await donation.save({ session });
await session.commitTransaction();
paymentsFailed.inc({ type: "donation", reason: "validation_failed" });

logger.error(`Donation ${donationId} validation failed:`, validationError.message);

return res.status(400).json({
success: false,
message: "Signed transaction does not match expected donation details",
error: validationError.message,
});
}

// Update status to submitted after validation
donation.status = "submitted";
donation.submittedAt = new Date();
await donation.save({ session });
Expand Down
52 changes: 49 additions & 3 deletions src/controllers/stellar/paymentController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
submitTransaction,
verifyTransaction,
verifyPaymentOperations,
validateSignedPaymentXdr,
NETWORK,
getExplorerUrl,
PLATFORM_WALLET_PUBLIC_KEY,
Expand Down Expand Up @@ -188,7 +189,8 @@ export const initializePayment = async (req, res) => {
network: NETWORK,
status: "pending",
settlement: settlementMode,
stellarTxHash: paymentTx.hash, // Temporary hash, will be replaced with actual
expectedHash: paymentTx.hash,
memo,
...(feeSplit && {
platformFee: {
feePercent: feeSplit.feePercent,
Expand Down Expand Up @@ -275,7 +277,51 @@ export const submitPayment = async (req, res) => {
});
}

// Update status to submitted
// Build expected payments to validate XDR BEFORE submit
let expectedPayments = transaction.platformFee?.platformAmount
? [
{
destination: transaction.creatorWallet,
amount: transaction.platformFee.creatorAmount,
},
{
destination: transaction.platformFee.platformWallet,
amount: transaction.platformFee.platformAmount,
},
]
: [
{
destination: transaction.creatorWallet,
amount: transaction.amount,
},
];
Comment on lines +280 to +297

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep the advertised SEP-7 flow aligned with fee-split validation.

For a fee split, this requires creator and platform transfers, but initialization still returns a SEP-7 URI for one full-price transfer. Wallets following that URI will be rejected for a missing platform payment, and the pending transaction is marked failed. Generate an equivalent multi-operation request, or omit the SEP-7 link when a split applies; add a regression test.

As per path instructions, “New or changed endpoints need Jest coverage.”

🤖 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/controllers/stellar/paymentController.js` around lines 280 - 297, Align
the SEP-7 initialization flow with the fee-split payments assembled in
expectedPayments: when transaction.platformFee is present, generate a
multi-operation request covering both creator and platform transfers, or omit
the SEP-7 URI. Preserve the single-transfer URI for transactions without a fee
split, and add Jest regression coverage for both behaviors.

Source: Path instructions


// Validate signed XDR contents (memo, payments, optional source)
try {
validateSignedPaymentXdr(
signedXdr,
expectedPayments,
transaction.memo,
transaction.buyerWallet,
true
);
} catch (validationError) {
transaction.status = "failed";
transaction.failureReason = `validation_failed: ${validationError.message}`;
await transaction.save({ session });
await session.commitTransaction();
paymentsFailed.inc({ type: "purchase", reason: "validation_failed" });

logger.error(`Transaction ${transactionId} validation failed:`, validationError.message);

return res.status(400).json({
success: false,
message: "Signed transaction does not match expected payment details",
error: validationError.message,
});
Comment on lines +317 to +321

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Use one validation-error envelope across Stellar submissions. Both new branches put error at the top level instead of the documented { success, message, data } shape.

  • src/controllers/stellar/paymentController.js#L317-L321: return validation detail as data.error (or data: null).
  • src/controllers/stellar/donationController.js#L181-L185: apply the same envelope.

As per path instructions, “New or changed endpoints need Jest coverage” and require consistent response shapes ({ success, message, data }).

📍 Affects 2 files
  • src/controllers/stellar/paymentController.js#L317-L321 (this comment)
  • src/controllers/stellar/donationController.js#L181-L185
🤖 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/controllers/stellar/paymentController.js` around lines 317 - 321, Update
the validation-error responses in paymentController.js at lines 317-321 and
donationController.js at lines 181-185 to use the documented { success, message,
data } envelope, placing validation details under data.error or using data:
null; add Jest coverage for both changed endpoint branches and preserve their
existing status and messages.

Source: Path instructions

}

// Update status to submitted after validation
transaction.status = "submitted";
transaction.submittedAt = new Date();
await transaction.save({ session });
Expand Down Expand Up @@ -304,7 +350,7 @@ export const submitPayment = async (req, res) => {

// Verify on-chain that the creator (and platform, when a fee was applied)
// actually received the expected USDC amounts
const expectedPayments = transaction.platformFee?.platformAmount
expectedPayments = transaction.platformFee?.platformAmount
? [
{
destination: transaction.creatorWallet,
Expand Down
32 changes: 25 additions & 7 deletions src/controllers/stellar/walletController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// controllers/stellar/walletController.js
import mongoose from "mongoose";
import User from "../../models/User.js";
import {
isValidPublicKey,
Expand Down Expand Up @@ -121,8 +122,11 @@ export const getWalletBalance = async (req, res) => {

res.status(200).json({
success: true,
publicKey,
...balance,
message: "Wallet balance fetched successfully",
data: {
publicKey,
...balance,
},
});
} catch (error) {
logger.error("Get wallet balance error:", error);
Expand Down Expand Up @@ -176,9 +180,21 @@ export const checkUserWallet = async (req, res) => {
try {
const { userId } = req.params;

const user = await User.findById(userId).select(
"stellarWallet.publicKey name"
);
if (!mongoose.Types.ObjectId.isValid(userId)) {
return res.status(400).json({
success: false,
message: "Invalid user ID",
});
}

if (userId !== req.user._id.toString()) {
return res.status(403).json({
success: false,
message: "Forbidden: You can only check your own wallet status",
});
}

const user = await User.findById(userId).select("stellarWallet.publicKey");

if (!user) {
return res.status(404).json({
Expand All @@ -189,8 +205,10 @@ export const checkUserWallet = async (req, res) => {

res.status(200).json({
success: true,
hasWallet: !!user.stellarWallet?.publicKey,
userName: user.name,
message: "Wallet status fetched successfully",
data: {
hasWallet: !!user.stellarWallet?.publicKey,
},
});
} catch (error) {
logger.error("Check user wallet error:", error);
Expand Down
21 changes: 15 additions & 6 deletions src/models/Transaction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,6 @@ import mongoose from "mongoose";
const transactionSchema = new mongoose.Schema(
{
// Transaction identification
stellarTxHash: {
type: String,
required: true,
unique: true,
index: true,
},
stellarLedger: {
type: Number,
},
Expand Down Expand Up @@ -46,6 +40,14 @@ const transactionSchema = new mongoose.Schema(
required: true,
},

// Transaction hash from the Stellar network, only set after confirmation
stellarTxHash: {
type: String,
unique: true,
sparse: true,
index: true,
},
Comment on lines +43 to +49

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
: "${MONGO_URI:?Set MONGO_URI to inspect the deployed database}"

mongosh "$MONGO_URI" --quiet --eval '
db.getCollectionNames().forEach((name) => {
  const indexes = db.getCollection(name).getIndexes()
    .filter((index) => index.key && index.key.stellarTxHash === 1);
  if (indexes.length) printjson({ collection: name, indexes });
});
'

Repository: Deen-Bridge/dnb-backend

Length of output: 233

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the Transaction model and any index-related migration/setup code.
git ls-files 'src/models/Transaction.js' 'src/**/*migration*' 'src/**/*index*' '.env.example' | sed -n '1,200p'
printf '\n--- Transaction.js outline ---\n'
ast-grep outline src/models/Transaction.js --view expanded
printf '\n--- relevant index/migration references ---\n'
rg -n "syncIndexes|createIndexes|ensureIndex|stellarTxHash|unique: true|sparse: true|autoIndex|index\(" src .env.example

Repository: Deen-Bridge/dnb-backend

Length of output: 2880

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n--- Transaction.js ---\n'
cat -n src/models/Transaction.js | sed -n '1,220p'

printf '\n--- startup / index syncing references ---\n'
rg -n "syncIndexes|ensureIndexes|ensureIndex|autoIndex|init\(\)|mongoose\.connect|set\('autoIndex'|dropIndexes|collection\.createIndex" src

Repository: Deen-Bridge/dnb-backend

Length of output: 5537

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,180p' src/config/db.js
printf '\n---\n'
rg -n "syncIndexes|ensureIndexes|dropIndex|dropIndexes|createIndex|autoIndex" src

Repository: Deen-Bridge/dnb-backend

Length of output: 3686

Rebuild the stellarTxHash index before shipping.
unique + sparse in the schema won’t change an existing collection, so any deployed non-sparse unique index will still reject multiple pending rows without a stellarTxHash. Add a migration to drop and recreate this index. src/models/Transaction.js:43-49

🤖 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/models/Transaction.js` around lines 43 - 49, add a migration that locates
the existing stellarTxHash index on the Transaction collection, drops it, and
recreates it as a unique sparse index matching the schema definition. Ensure the
migration is safe to run against deployments where the index is absent or
already correctly configured, and leave the stellarTxHash schema unchanged.

Source: Path instructions


// Item being purchased (not applicable to donations)
itemType: {
type: String,
Expand Down Expand Up @@ -90,6 +92,13 @@ const transactionSchema = new mongoose.Schema(
enum: ["testnet", "mainnet"],
required: true,
},
expectedHash: {
type: String,
index: true,
},
memo: {
type: String,
},

// Platform fee split (only set when a fee was applied at build time)
platformFee: {
Expand Down
6 changes: 3 additions & 3 deletions src/routes/stellar/walletRoutes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ router.post("/connect", protect, connectWallet);
router.delete("/disconnect", protect, disconnectWallet);
router.get("/me", protect, getMyWallet);

// Public routes
router.get("/balance/:publicKey", getWalletBalance);
router.get("/check/:userId", checkUserWallet);
// Lookup routes require authentication to avoid wallet/user enumeration.
router.get("/balance/:publicKey", protect, getWalletBalance);
router.get("/check/:userId", protect, checkUserWallet);

export default router;
65 changes: 65 additions & 0 deletions src/services/stellar/stellarService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,71 @@ export const submitTransaction = async (signedXdr) => {
}
};

/**
* Validate a signed transaction XDR against expected payments and memo/source
* @param {string} signedXdr
* @param {Array<{destination:string, amount:string}>} expectedPayments
* @param {string} expectedMemo
* @param {string} expectedSource
* @param {boolean} requireSource
*/
export const validateSignedPaymentXdr = (
signedXdr,
expectedPayments = [],
expectedMemo,
expectedSource,
requireSource = true
) => {
const tx = StellarSdk.TransactionBuilder.fromXDR(
signedXdr,
networkPassphrase
);

// Memo check
if (expectedMemo) {
const memo = tx.memo;
let memoText = null;
if (memo && memo._type === "text") {
const val = memo._value;
memoText = Buffer.isBuffer(val) ? val.toString() : String(val);
}
if (memoText !== expectedMemo) {
throw new Error("Memo mismatch");
}
}

// Source check
if (requireSource && expectedSource) {
if (tx.source !== expectedSource) {
throw new Error("Source account mismatch");
}
}

// Payment operations check
const paymentOps = tx.operations.filter((op) => op.type === "payment");

for (const expected of expectedPayments) {
const match = paymentOps.find((op) => {
const assetMatches =
(op.asset && op.asset.code === "USDC" && op.asset.issuer === USDC_ISSUER) ||
(op.asset_type === "credit_alphanum4" && op.asset?.code === "USDC" && op.asset?.issuer === USDC_ISSUER);

const amountMatches = toStroops(op.amount) === toStroops(expected.amount);
const destMatches = op.destination === expected.destination;

return assetMatches && amountMatches && destMatches;
});

if (!match) {
throw new Error(
`Signed XDR missing expected USDC payment of ${expected.amount} to ${expected.destination}`
);
}
}

return tx;
};

export const verifyTransaction = async (txHash) => {
try {
const tx = await timedHorizonCall("fetchTransaction", () =>
Expand Down
8 changes: 8 additions & 0 deletions test/jest.setup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { MongoMemoryServer } from 'mongodb-memory-server';

process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-secret';

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Move test configuration out of JavaScript. Both sites silently inject configuration rather than requiring the documented test environment, allowing CI to run with an undeclared JWT secret or database target.

  • test/jest.setup.js#L3-L3: remove the inline JWT fallback and require JWT_SECRET from the test environment.
  • src/config/validateEnv.js#L34-L40: remove the inline JWT/Mongo defaults; validate the documented test environment instead of replacing it.

As per path instructions, “all configuration belongs in environment variables documented in .env.example.”

📍 Affects 2 files
  • test/jest.setup.js#L3-L3 (this comment)
  • src/config/validateEnv.js#L34-L40
🤖 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 `@test/jest.setup.js` at line 3, Remove the JWT_SECRET fallback from
test/jest.setup.js so tests require the documented environment variable. In
src/config/validateEnv.js, remove the inline JWT and Mongo defaults and validate
the documented test environment values directly, keeping all configuration
sourced from variables defined in .env.example.

Source: Path instructions


// Start an in-memory MongoDB for tests
const mongod = await MongoMemoryServer.create();
process.env.MONGO_URI = mongod.getUri();
global.__MONGOD__ = mongod;
Comment on lines +6 to +8

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 'MongoMemoryServer|__MONGOD__|globalTeardown|setupFilesAfterEnv|\.stop\(' \
  jest.config.js test src

Repository: Deen-Bridge/dnb-backend

Length of output: 614

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## files\n'
git ls-files | rg '(^|/)(jest\.config\..*|package\.json|test/|src/)' | sed -n '1,200p'

printf '\n## teardown-related search\n'
rg -n -C3 'MongoMemoryServer|__MONGOD__|globalTeardown|setupFilesAfterEnv|setupFiles|teardown|\.stop\(' .

printf '\n## jest setup file\n'
cat -n test/jest.setup.js

printf '\n## jest config candidates\n'
for f in $(git ls-files | rg '(^|/)jest\.config\..*$|(^|/)package\.json$'); do
  echo "--- $f"
  cat -n "$f" | sed -n '1,220p'
done

Repository: Deen-Bridge/dnb-backend

Length of output: 5716

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## mongoose connection handling in tests\n'
rg -n -C2 'mongoose\.connect|mongoose\.disconnect|disconnect\(|close\(\)|__MONGOD__|MongoMemoryServer' test src

printf '\n## test bootstrap files\n'
for f in test/*.js; do
  echo "--- $f"
  cat -n "$f" | sed -n '1,240p'
done

Repository: Deen-Bridge/dnb-backend

Length of output: 44250

Move the MongoMemoryServer lifecycle to teardown.
test/jest.setup.js starts MongoMemoryServer.create() from setupFiles, so each test file spins up a new MongoDB instance and none of them are stopped. That leaves child processes alive until Jest force-exits and adds avoidable CI overhead. Move this to globalSetup/globalTeardown, or stop global.__MONGOD__ in teardown.

🤖 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 `@test/jest.setup.js` around lines 6 - 8, Move the MongoMemoryServer startup
currently in the setup initialization to a Jest global setup/teardown lifecycle,
ensuring one shared instance is created for the test run and explicitly stopped
afterward. Update the MONGO_URI propagation and shared instance handling so
tests continue using the started server without leaving child processes running.

Loading