Skip to content

Commit e7c95aa

Browse files
committed
fix: address CodeRabbit feedback for DB validation
1 parent c5a8b4e commit e7c95aa

6 files changed

Lines changed: 215 additions & 156 deletions

File tree

‎apps/dashboard-api/src/controllers/project.controller.js‎

Lines changed: 46 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const { encrypt, decrypt } = require("@urbackend/common");
2222
const { URL } = require("url");
2323
const path = require("path");
2424
const axios = require("axios");
25-
const { getConnection } = require("@urbackend/common");
25+
const { getConnection, isSafeUri } = require("@urbackend/common");
2626
const { getCompiledModel } = require("@urbackend/common");
2727
const { QueryEngine } = require("@urbackend/common");
2828
const { storageRegistry } = require("@urbackend/common");
@@ -585,123 +585,6 @@ const dropCollectionIfExists = async (connection, collectionName) => {
585585
}
586586
};
587587

588-
/**
589-
* Convert a dotted-decimal IPv4 address string to a 32-bit unsigned integer.
590-
* @param {string} ip - IPv4 address in dotted-decimal notation (e.g., '192.168.1.1')
591-
* @returns {number} 32-bit unsigned integer representation of the IPv4 address
592-
*/
593-
function ipv4ToInt(ip) {
594-
return ip.split(".").reduce((acc, octet) => (acc << 8) + parseInt(octet, 10), 0) >>> 0;
595-
}
596-
597-
/**
598-
* Check if an IPv4 address falls within any restricted or reserved IP range.
599-
* Blocks loopback (127.0.0.0/8), RFC-1918 private ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16),
600-
* link-local and cloud metadata (169.254.0.0/16), and unspecified (0.0.0.0/8) addresses to prevent SSRF attacks.
601-
* @param {string} ip - IPv4 address to validate
602-
* @returns {boolean} True if the IP is restricted, false otherwise
603-
*/
604-
function isRestrictedIPv4(ip) {
605-
const n = ipv4ToInt(ip);
606-
// Loopback 127.0.0.0/8
607-
if (n >= ipv4ToInt("127.0.0.0") && n <= ipv4ToInt("127.255.255.255")) return true;
608-
// RFC-1918: 10.0.0.0/8
609-
if (n >= ipv4ToInt("10.0.0.0") && n <= ipv4ToInt("10.255.255.255")) return true;
610-
// RFC-1918: 172.16.0.0/12
611-
if (n >= ipv4ToInt("172.16.0.0") && n <= ipv4ToInt("172.31.255.255")) return true;
612-
// RFC-1918: 192.168.0.0/16
613-
if (n >= ipv4ToInt("192.168.0.0") && n <= ipv4ToInt("192.168.255.255")) return true;
614-
// Link-local / cloud instance metadata (AWS, GCP, Azure): 169.254.0.0/16
615-
if (n >= ipv4ToInt("169.254.0.0") && n <= ipv4ToInt("169.254.255.255")) return true;
616-
// Unspecified: 0.0.0.0/8
617-
if (n >= ipv4ToInt("0.0.0.0") && n <= ipv4ToInt("0.255.255.255")) return true;
618-
return false;
619-
}
620-
621-
/**
622-
* Check if an IPv6 address falls within any restricted or reserved range.
623-
* Blocks loopback (::1), unspecified (::), link-local (fe80::/10), IPv6 Unique Local Addresses (fc00::/7),
624-
* and IPv4-mapped IPv6 addresses that resolve to restricted IPv4 ranges to prevent SSRF attacks.
625-
* @param {string} ip - IPv6 address to validate
626-
* @returns {boolean} True if the IP is restricted, false otherwise
627-
*/
628-
function isRestrictedIPv6(ip) {
629-
const expanded = ip.replace(/^\[|\]$/g, "").toLowerCase();
630-
// IPv6 loopback ::1
631-
if (expanded === "::1" || expanded === "0:0:0:0:0:0:0:1") return true;
632-
// IPv6 unspecified ::
633-
if (expanded === "::" || expanded === "0:0:0:0:0:0:0:0") return true;
634-
// IPv4-mapped IPv6 addresses (::ffff:x.x.x.x)
635-
const mapped = expanded.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/);
636-
if (mapped && isRestrictedIPv4(mapped[1])) return true;
637-
// IPv6 link-local fe80::/10 (fe80: through febf:)
638-
if (/^fe[89ab]/.test(expanded)) return true;
639-
// IPv6 ULA (Unique Local Address) fc00::/7
640-
if (expanded.startsWith("fc") || expanded.startsWith("fd")) return true;
641-
return false;
642-
}
643-
644-
/**
645-
* Check if an IP address (either IPv4 or IPv6) is restricted or falls within a reserved range.
646-
* Delegates to isRestrictedIPv4() or isRestrictedIPv6() based on the IP version.
647-
* @param {string} ip - IP address to validate
648-
* @returns {boolean} True if the IP is restricted, false otherwise
649-
*/
650-
function isRestrictedIP(ip) {
651-
if (net.isIPv4(ip)) return isRestrictedIPv4(ip);
652-
if (net.isIPv6(ip)) return isRestrictedIPv6(ip);
653-
return false;
654-
}
655-
656-
/**
657-
* Validate a URI to ensure it does not target restricted hosts or IP ranges (SSRF prevention).
658-
* Performs DNS resolution on hostnames to check both A and AAAA records against restricted ranges.
659-
* Blocks loopback, RFC-1918 private ranges, cloud metadata endpoints, and other reserved IP ranges.
660-
* @async
661-
* @param {string} uri - The URI to validate
662-
* @returns {Promise<boolean>} True if the URI is safe to connect to, false if it targets a restricted host
663-
*/
664-
const isSafeUri = async (uri) => {
665-
try {
666-
const parsed = new URL(uri);
667-
const host = parsed.hostname.replace(/^\[|\]$/g, "").toLowerCase();
668-
669-
// Block well-known loopback and internal hostnames
670-
const blockedHostnames = ["localhost", "metadata.google.internal"];
671-
if (blockedHostnames.includes(host)) return false;
672-
673-
// Reject mongodb+srv:// URIs as they perform hidden SRV/TXT discovery
674-
// We cannot safely validate all targets without resolving SRV records
675-
if (uri.toLowerCase().includes("mongodb+srv://")) return false;
676-
677-
// If the host is a bare IPv4 or IPv6 address, check all restricted ranges
678-
if (net.isIPv4(host) || net.isIPv6(host)) {
679-
return !isRestrictedIP(host);
680-
}
681-
682-
// For hostnames, perform DNS resolution to check both A and AAAA records
683-
const [ipv4Result, ipv6Result] = await Promise.allSettled([
684-
dns.resolve4(host),
685-
dns.resolve6(host),
686-
]);
687-
688-
const resolved = [
689-
...(ipv4Result.status === "fulfilled" ? ipv4Result.value : []),
690-
...(ipv6Result.status === "fulfilled" ? ipv6Result.value : []),
691-
];
692-
693-
// If no addresses resolved, treat as unsafe
694-
if (resolved.length === 0) return false;
695-
696-
// Check all resolved addresses for restricted ranges
697-
if (resolved.some((addr) => isRestrictedIP(addr))) return false;
698-
699-
return true;
700-
} catch (e) {
701-
return false;
702-
}
703-
};
704-
705588
module.exports.updateExternalConfig = async (req, res) => {
706589
try {
707590
const { projectId } = req.params;
@@ -726,48 +609,66 @@ module.exports.updateExternalConfig = async (req, res) => {
726609

727610
let dashboardIp = null;
728611
let publicApiIp = null;
612+
let tempConn = null;
729613

730614
try {
731-
dashboardIp = await getPublicIp();
732-
733-
// 1. Test local connection (Dashboard API)
734-
const tempConn = mongoose.createConnection(dbUri, {
735-
serverSelectionTimeoutMS: 5000,
736-
});
737-
await tempConn.asPromise();
738-
await tempConn.close();
615+
await Promise.race([
616+
new Promise((_, reject) => setTimeout(() => reject(new Error("Overall verification timeout exceeded")), 9500)),
617+
(async () => {
618+
// 1. Test local connection (Dashboard API)
619+
tempConn = mongoose.createConnection(dbUri, {
620+
serverSelectionTimeoutMS: 5000,
621+
});
622+
await tempConn.asPromise();
739623

740-
// 2. Test remote connection (Public API) if PUBLIC_API_URL is configured
741-
if (process.env.PUBLIC_API_URL && process.env.INTERNAL_SECRET) {
742-
const publicApiRes = await axios.post(`${process.env.PUBLIC_API_URL}/api/internal/test-db`,
624+
// 2. Test remote connection (Public API)
625+
if (!process.env.PUBLIC_API_URL || !process.env.INTERNAL_SECRET) {
626+
throw new Error("Missing PUBLIC_API_URL or INTERNAL_SECRET for DB verification.");
627+
}
628+
629+
await axios.post(`${process.env.PUBLIC_API_URL}/api/internal/test-db`,
743630
{ dbUri },
744631
{ headers: { 'x-internal-secret': process.env.INTERNAL_SECRET }, timeout: 8000 }
745632
);
746-
// If it didn't throw, it succeeded.
747-
} else {
748-
console.warn("Skipping Public API DB verification because PUBLIC_API_URL or INTERNAL_SECRET is missing.");
749-
}
750-
633+
})()
634+
]);
751635
} catch (connErr) {
752636
console.error("Verification Connection Failed:", connErr.message);
753637
let errorMsg = "Could not connect to the provided MongoDB URI.";
754638

639+
try {
640+
dashboardIp = await getPublicIp();
641+
} catch (ipErr) {
642+
console.error("Failed to fetch Dashboard API IP:", ipErr.message);
643+
}
644+
755645
// If the error came from Axios (Public API verification failed)
756-
if (connErr.response && connErr.response.data && !connErr.response.data.success) {
757-
errorMsg = connErr.response.data.message;
758-
if (connErr.response.data.serverIp) {
759-
publicApiIp = connErr.response.data.serverIp;
646+
if (connErr.response) {
647+
if (connErr.response.data && connErr.response.data.success === false && connErr.response.status === 400) {
648+
// It's a database-URI-specific verification failure
649+
errorMsg = connErr.response.data.message;
650+
if (connErr.response.data.data && connErr.response.data.data.serverIp) {
651+
publicApiIp = connErr.response.data.data.serverIp;
652+
} else if (connErr.response.data.serverIp) {
653+
publicApiIp = connErr.response.data.serverIp;
654+
}
655+
} else {
656+
// Infrastructure error
657+
errorMsg = "Public API verification failed.";
760658
}
761659
} else if (
762660
connErr.message.includes("Server selection timed out") ||
763-
connErr.message.includes("Could not connect")
661+
connErr.message.includes("Could not connect") ||
662+
connErr.message.includes("Overall verification timeout exceeded")
764663
) {
765664
// Local failure
766665
errorMsg = `Access Denied: Please whitelist Server IPs in MongoDB Atlas.`;
666+
} else if (connErr.message.includes("Missing PUBLIC_API_URL")) {
667+
return res.status(500).json({ success: false, data: {}, message: "Server misconfiguration: DB verification failed." });
767668
}
768669

769670
// Fetch Public API IP if we don't have it yet, to show both in the error message
770-
if (!publicApiIp && process.env.PUBLIC_API_URL) {
671+
if (!publicApiIp && process.env.PUBLIC_API_URL && errorMsg.includes("whitelist Server IPs")) {
771672
try {
772673
const ipRes = await axios.get(`${process.env.PUBLIC_API_URL}/api/server-ip`, { timeout: 3000 });
773674
if (ipRes.data && ipRes.data.ip) publicApiIp = ipRes.data.ip;
@@ -776,12 +677,16 @@ module.exports.updateExternalConfig = async (req, res) => {
776677
}
777678
}
778679

779-
if (dashboardIp || publicApiIp) {
680+
if (errorMsg.includes("whitelist Server IPs") && (dashboardIp || publicApiIp)) {
780681
const ips = [dashboardIp, publicApiIp].filter(Boolean).join(", ");
781682
errorMsg = `Access Denied: Please whitelist Server IPs [${ips}] in MongoDB Atlas.`;
782683
}
783684

784685
return res.status(400).json({ success: false, data: {}, message: errorMsg });
686+
} finally {
687+
if (tempConn) {
688+
await tempConn.close();
689+
}
785690
}
786691
}
787692

‎apps/public-api/src/app.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ app.use('/api/storage', limiter, logger, storageRoute);
7979
app.use('/api/mail', limiter, logger, mailRoute);
8080
app.use('/api/health', limiter, logger, healthRoute);
8181

82+
// IMPORTANT: Restrict the /api/internal route exposed below to trusted internal network sources
83+
// at the infrastructure layer (e.g., VPC, Security Groups, or ingress rules).
84+
// Ensure this path is unreachable from the public internet.
8285
const internalRoute = require('./routes/internal');
8386
app.use('/api/internal', internalRoute);
8487

‎apps/public-api/src/routes/internal.js‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
11
const express = require('express');
22
const mongoose = require('mongoose');
3-
const { getPublicIp } = require('@urbackend/common');
3+
const crypto = require('crypto');
4+
const { getPublicIp, isSafeUri } = require('@urbackend/common');
45
const router = express.Router();
56

67
// Middleware to protect internal routes
78
const internalAuth = (req, res, next) => {
89
const secret = process.env.INTERNAL_SECRET;
910
if (!secret) {
1011
console.error("INTERNAL_SECRET is not configured on Public API");
11-
return res.status(500).json({ success: false, message: "Server misconfiguration" });
12+
return res.status(500).json({ success: false, data: {}, message: "Server misconfiguration" });
1213
}
1314

14-
const providedSecret = req.headers['x-internal-secret'];
15-
if (!providedSecret || providedSecret !== secret) {
16-
return res.status(403).json({ success: false, message: "Forbidden: Invalid internal secret" });
15+
const providedSecret = req.headers['x-internal-secret'] || '';
16+
17+
// Convert both to buffers of the same length to use timingSafeEqual
18+
const secretBuffer = Buffer.from(secret, 'utf8');
19+
const providedBuffer = Buffer.from(providedSecret, 'utf8');
20+
21+
if (secretBuffer.length !== providedBuffer.length || !crypto.timingSafeEqual(secretBuffer, providedBuffer)) {
22+
return res.status(403).json({ success: false, data: {}, message: "Forbidden: Invalid internal secret" });
1723
}
1824

1925
next();
@@ -22,17 +28,25 @@ const internalAuth = (req, res, next) => {
2228
router.post('/test-db', internalAuth, async (req, res) => {
2329
const { dbUri } = req.body;
2430
if (!dbUri) {
25-
return res.status(400).json({ success: false, message: "dbUri is required" });
31+
return res.status(400).json({ success: false, data: {}, message: "dbUri is required" });
32+
}
33+
34+
if (!(await isSafeUri(dbUri))) {
35+
return res.status(400).json({
36+
success: false,
37+
data: {},
38+
message: "DB URI is pointing to a restricted host, internal network, or unsupported URI format.",
39+
});
2640
}
2741

2842
console.log("[Internal] Verifying connection for external DB...");
43+
let tempConn = null;
2944
try {
30-
const tempConn = mongoose.createConnection(dbUri, {
45+
tempConn = mongoose.createConnection(dbUri, {
3146
serverSelectionTimeoutMS: 5000,
3247
});
3348
await tempConn.asPromise();
34-
await tempConn.close();
35-
return res.status(200).json({ success: true, message: "Connection verified" });
49+
return res.status(200).json({ success: true, data: {}, message: "Connection verified" });
3650
} catch (connErr) {
3751
console.error("[Internal] Verification Connection Failed:", connErr.message);
3852
let errorMsg = "Could not connect to the provided MongoDB URI.";
@@ -42,11 +56,19 @@ router.post('/test-db', internalAuth, async (req, res) => {
4256
connErr.message.includes("Server selection timed out") ||
4357
connErr.message.includes("Could not connect")
4458
) {
45-
serverIp = await getPublicIp();
46-
errorMsg = `Access Denied: Please whitelist Server IP [${serverIp}] in MongoDB Atlas.`;
59+
try {
60+
serverIp = await getPublicIp();
61+
errorMsg = `Access Denied: Please whitelist Server IP [${serverIp}] in MongoDB Atlas.`;
62+
} catch (ipErr) {
63+
console.error("Failed to fetch Public API IP:", ipErr.message);
64+
}
4765
}
4866

49-
return res.status(400).json({ success: false, message: errorMsg, serverIp });
67+
return res.status(400).json({ success: false, data: { serverIp }, message: errorMsg });
68+
} finally {
69+
if (tempConn) {
70+
await tempConn.close();
71+
}
5072
}
5173
});
5274

‎apps/web-dashboard/src/components/Settings/DatabaseConfigForm.jsx‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,17 @@ export default function DatabaseConfigForm({ project, projectId, onProjectUpdate
2121
const fetchIp = async () => {
2222
try {
2323
const { PUBLIC_API_URL } = await import('../../config');
24+
const controller = new AbortController();
25+
const timeoutId = setTimeout(() => controller.abort(), 3000);
26+
2427
const [res1, res2] = await Promise.allSettled([
2528
api.get(`/api/server-ip`),
26-
fetch(`${PUBLIC_API_URL}/api/server-ip`).then(r => r.json())
29+
fetch(`${PUBLIC_API_URL}/api/server-ip`, { signal: controller.signal })
30+
.then(r => {
31+
if (!r.ok) throw new Error("Public API unresponsive");
32+
return r.json();
33+
})
34+
.finally(() => clearTimeout(timeoutId))
2735
]);
2836

2937
const ips = [];

‎packages/common/src/index.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ const {
107107
clearCompiledModel,
108108
createUniqueIndexes,
109109
} = require("./utils/injectModel");
110-
const { getPublicIp } = require("./utils/network");
110+
const { getPublicIp, isSafeUri } = require("./utils/network");
111111
const {
112112
isProjectStorageExternal,
113113
isProjectDbExternal,
@@ -198,6 +198,7 @@ module.exports = {
198198
clearCompiledModel,
199199
createUniqueIndexes,
200200
getPublicIp,
201+
isSafeUri,
201202
isProjectStorageExternal,
202203
isProjectDbExternal,
203204
getBucket,

0 commit comments

Comments
 (0)