Skip to content

Commit 4edec9b

Browse files
authored
Merge pull request #7 from fcsonline/fix/security-critical-high
fix: patch critical and high security vulnerabilities
2 parents 08c9f4c + 9fd93b8 commit 4edec9b

9 files changed

Lines changed: 168 additions & 70 deletions

File tree

Dockerfile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ COPY packages/shared/package.json ./packages/shared/
1111
COPY packages/backend/package.json ./packages/backend/
1212
COPY packages/frontend/package.json ./packages/frontend/
1313

14-
# Install dependencies
15-
RUN npm install
14+
# Install dependencies (skip prepare/lefthook — not needed in Docker)
15+
RUN npm install --ignore-scripts
1616

1717
# Copy source
1818
COPY packages/shared/ ./packages/shared/
@@ -41,7 +41,7 @@ COPY packages/shared/package.json ./packages/shared/
4141
COPY packages/backend/package.json ./packages/backend/
4242
COPY packages/frontend/package.json ./packages/frontend/
4343

44-
RUN npm install --omit=dev
44+
RUN npm install --omit=dev --ignore-scripts
4545

4646
# Copy shared compiled output (needed at runtime for imports)
4747
COPY packages/shared/package.json ./packages/shared/

packages/backend/src/index.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,18 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url));
1414
const app = express();
1515
const PORT = process.env.PORT || 3001;
1616

17-
app.use(cors());
17+
// CORS configuration — restrict to configured origins in production
18+
const corsOrigin = process.env.CORS_ORIGIN;
19+
app.use(
20+
cors(
21+
corsOrigin
22+
? {
23+
origin: corsOrigin.split(",").map((o) => o.trim()),
24+
credentials: true,
25+
}
26+
: undefined,
27+
),
28+
);
1829
app.use(express.json({ limit: "50mb" }));
1930

2031
// Serve frontend static files in production
@@ -44,5 +55,7 @@ app.listen(PORT, () => {
4455
console.log(`DroneRoute server running on http://localhost:${PORT}`);
4556
const selfHosted = (process.env.SELF_HOSTED ?? "true") === "true";
4657
const adminEmail = process.env.ADMIN_EMAIL || "";
47-
console.log(`Mode: ${selfHosted ? "self-hosted" : "cloud"}${!selfHosted && adminEmail ? ` (admin: ${adminEmail})` : ""}`);
58+
console.log(
59+
`Mode: ${selfHosted ? "self-hosted" : "cloud"}${!selfHosted && adminEmail ? ` (admin: ${adminEmail})` : ""}`,
60+
);
4861
});

packages/backend/src/middleware/auth.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,17 @@ export function authMiddleware(
2727

2828
// Check if user is banned
2929
const db = getDb();
30-
const user = db.prepare("SELECT is_banned, is_admin FROM users WHERE id = ?").get(payload.userId) as any;
30+
const user = db
31+
.prepare("SELECT is_banned, is_admin FROM users WHERE id = ?")
32+
.get(payload.userId) as any;
3133
if (!user) {
3234
res.status(401).json({ error: "User not found" });
3335
return;
3436
}
3537
if (user.is_banned) {
36-
res.status(403).json({ error: "Your account has been suspended", banned: true });
38+
res
39+
.status(403)
40+
.json({ error: "Your account has been suspended", banned: true });
3741
return;
3842
}
3943

packages/backend/src/models/db.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,18 @@ export function initDb(): void {
7777

7878
// Migration: add is_admin column if missing (for existing DBs)
7979
try {
80-
database.exec(`ALTER TABLE users ADD COLUMN is_admin INTEGER NOT NULL DEFAULT 0`);
80+
database.exec(
81+
`ALTER TABLE users ADD COLUMN is_admin INTEGER NOT NULL DEFAULT 0`,
82+
);
8183
} catch {
8284
// Column already exists — ignore
8385
}
8486

8587
// Migration: add is_banned column if missing (for existing DBs)
8688
try {
87-
database.exec(`ALTER TABLE users ADD COLUMN is_banned INTEGER NOT NULL DEFAULT 0`);
89+
database.exec(
90+
`ALTER TABLE users ADD COLUMN is_banned INTEGER NOT NULL DEFAULT 0`,
91+
);
8892
} catch {
8993
// Column already exists — ignore
9094
}
@@ -93,7 +97,9 @@ export function initDb(): void {
9397
const selfHosted = (process.env.SELF_HOSTED ?? "true") === "true";
9498
const adminEmail = process.env.ADMIN_EMAIL || "";
9599
if (!selfHosted && adminEmail) {
96-
database.prepare("UPDATE users SET is_admin = 1 WHERE LOWER(email) = LOWER(?)").run(adminEmail);
100+
database
101+
.prepare("UPDATE users SET is_admin = 1 WHERE LOWER(email) = LOWER(?)")
102+
.run(adminEmail);
97103
}
98104

99105
console.log("Database initialized at", DB_PATH);

packages/backend/src/routes/admin.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ function adminGuard(req: AuthRequest, res: Response, next: NextFunction): void {
1717
}
1818

1919
const db = getDb();
20-
const user = db.prepare("SELECT is_admin FROM users WHERE id = ?").get(req.userId) as any;
20+
const user = db
21+
.prepare("SELECT is_admin FROM users WHERE id = ?")
22+
.get(req.userId) as any;
2123

2224
if (!user || !user.is_admin) {
2325
res.status(403).json({ error: "Admin access required" });
@@ -33,12 +35,16 @@ adminRoutes.use(authMiddleware, adminGuard);
3335
// GET /api/admin/users?page=1&perPage=20
3436
adminRoutes.get("/users", (req: AuthRequest, res) => {
3537
const page = Math.max(1, parseInt(req.query.page as string) || 1);
36-
const perPage = Math.min(100, Math.max(1, parseInt(req.query.perPage as string) || 20));
38+
const perPage = Math.min(
39+
100,
40+
Math.max(1, parseInt(req.query.perPage as string) || 20),
41+
);
3742
const offset = (page - 1) * perPage;
3843

3944
const db = getDb();
4045

41-
const total = (db.prepare("SELECT COUNT(*) as count FROM users").get() as any).count;
46+
const total = (db.prepare("SELECT COUNT(*) as count FROM users").get() as any)
47+
.count;
4248

4349
const users = db
4450
.prepare(
@@ -48,7 +54,7 @@ adminRoutes.get("/users", (req: AuthRequest, res) => {
4854
LEFT JOIN missions m ON m.user_id = u.id
4955
GROUP BY u.id
5056
ORDER BY u.created_at DESC
51-
LIMIT ? OFFSET ?`
57+
LIMIT ? OFFSET ?`,
5258
)
5359
.all(perPage, offset) as any[];
5460

@@ -75,7 +81,9 @@ adminRoutes.post("/users/:id/ban", (req: AuthRequest, res) => {
7581
}
7682

7783
const db = getDb();
78-
const result = db.prepare("UPDATE users SET is_banned = 1 WHERE id = ?").run(req.params.id);
84+
const result = db
85+
.prepare("UPDATE users SET is_banned = 1 WHERE id = ?")
86+
.run(req.params.id);
7987
if (result.changes === 0) {
8088
res.status(404).json({ error: "User not found" });
8189
return;
@@ -91,7 +99,9 @@ adminRoutes.post("/users/:id/unban", (req: AuthRequest, res) => {
9199
}
92100

93101
const db = getDb();
94-
const result = db.prepare("UPDATE users SET is_banned = 0 WHERE id = ?").run(req.params.id);
102+
const result = db
103+
.prepare("UPDATE users SET is_banned = 0 WHERE id = ?")
104+
.run(req.params.id);
95105
if (result.changes === 0) {
96106
res.status(404).json({ error: "User not found" });
97107
return;
@@ -107,7 +117,9 @@ adminRoutes.post("/users/:id/promote", (req: AuthRequest, res) => {
107117
}
108118

109119
const db = getDb();
110-
const result = db.prepare("UPDATE users SET is_admin = 1 WHERE id = ?").run(req.params.id);
120+
const result = db
121+
.prepare("UPDATE users SET is_admin = 1 WHERE id = ?")
122+
.run(req.params.id);
111123
if (result.changes === 0) {
112124
res.status(404).json({ error: "User not found" });
113125
return;
@@ -123,7 +135,9 @@ adminRoutes.post("/users/:id/demote", (req: AuthRequest, res) => {
123135
}
124136

125137
const db = getDb();
126-
const result = db.prepare("UPDATE users SET is_admin = 0 WHERE id = ?").run(req.params.id);
138+
const result = db
139+
.prepare("UPDATE users SET is_admin = 0 WHERE id = ?")
140+
.run(req.params.id);
127141
if (result.changes === 0) {
128142
res.status(404).json({ error: "User not found" });
129143
return;

packages/backend/src/routes/kmz.ts

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@ import { DEFAULT_MISSION_CONFIG } from "@droneroute/shared";
66
import { generateKmzBuffer } from "../services/kmzGenerator.js";
77
import { parseKmz } from "../services/kmzParser.js";
88
import { getDb } from "../models/db.js";
9-
import { optionalAuth, type AuthRequest } from "../middleware/auth.js";
9+
import {
10+
authMiddleware,
11+
optionalAuth,
12+
type AuthRequest,
13+
} from "../middleware/auth.js";
1014

1115
export const kmzRoutes = Router();
1216

@@ -16,7 +20,7 @@ const upload = multer({
1620
});
1721

1822
// Generate and download KMZ from mission data (POST body)
19-
kmzRoutes.post("/generate", async (req, res) => {
23+
kmzRoutes.post("/generate", authMiddleware, async (req: AuthRequest, res) => {
2024
try {
2125
const { name, config, waypoints, pois } = req.body;
2226
if (!config || !waypoints || waypoints.length < 2) {
@@ -44,45 +48,52 @@ kmzRoutes.post("/generate", async (req, res) => {
4448
res.setHeader("Content-Disposition", `attachment; filename="${filename}"`);
4549
res.send(buffer);
4650
} catch (err: any) {
47-
console.error("KMZ generation error:", err);
48-
res.status(500).json({ error: err.message || "Failed to generate KMZ" });
51+
console.error("KMZ download error:", err);
52+
res.status(500).json({ error: "Failed to generate KMZ" });
4953
}
5054
});
5155

5256
// Download KMZ for a saved mission
53-
kmzRoutes.get("/download/:missionId", async (req, res) => {
54-
try {
55-
const db = getDb();
56-
const row = db
57-
.prepare("SELECT * FROM missions WHERE id = ?")
58-
.get(req.params.missionId) as any;
59-
if (!row) {
60-
res.status(404).json({ error: "Mission not found" });
61-
return;
62-
}
57+
kmzRoutes.get(
58+
"/download/:missionId",
59+
authMiddleware,
60+
async (req: AuthRequest, res) => {
61+
try {
62+
const db = getDb();
63+
const row = db
64+
.prepare("SELECT * FROM missions WHERE id = ? AND user_id = ?")
65+
.get(req.params.missionId, req.userId) as any;
66+
if (!row) {
67+
res.status(404).json({ error: "Mission not found" });
68+
return;
69+
}
6370

64-
const mission: Mission = {
65-
id: row.id,
66-
name: row.name,
67-
userId: row.user_id,
68-
createdAt: row.created_at,
69-
updatedAt: row.updated_at,
70-
config: JSON.parse(row.config),
71-
waypoints: JSON.parse(row.waypoints),
72-
pois: JSON.parse(row.pois || "[]"),
73-
obstacles: JSON.parse(row.obstacles || "[]"),
74-
};
71+
const mission: Mission = {
72+
id: row.id,
73+
name: row.name,
74+
userId: row.user_id,
75+
createdAt: row.created_at,
76+
updatedAt: row.updated_at,
77+
config: JSON.parse(row.config),
78+
waypoints: JSON.parse(row.waypoints),
79+
pois: JSON.parse(row.pois || "[]"),
80+
obstacles: JSON.parse(row.obstacles || "[]"),
81+
};
7582

76-
const buffer = await generateKmzBuffer(mission);
77-
const filename = `${mission.name.replace(/[^a-zA-Z0-9_-]/g, "_")}.kmz`;
78-
res.setHeader("Content-Type", "application/vnd.google-earth.kmz");
79-
res.setHeader("Content-Disposition", `attachment; filename="${filename}"`);
80-
res.send(buffer);
81-
} catch (err: any) {
82-
console.error("KMZ download error:", err);
83-
res.status(500).json({ error: err.message || "Failed to generate KMZ" });
84-
}
85-
});
83+
const buffer = await generateKmzBuffer(mission);
84+
const filename = `${mission.name.replace(/[^a-zA-Z0-9_-]/g, "_")}.kmz`;
85+
res.setHeader("Content-Type", "application/vnd.google-earth.kmz");
86+
res.setHeader(
87+
"Content-Disposition",
88+
`attachment; filename="${filename}"`,
89+
);
90+
res.send(buffer);
91+
} catch (err: any) {
92+
console.error("KMZ download error:", err);
93+
res.status(500).json({ error: err.message || "Failed to generate KMZ" });
94+
}
95+
},
96+
);
8697

8798
// Import KMZ file
8899
kmzRoutes.post(
@@ -123,7 +134,7 @@ kmzRoutes.post(
123134
res.json({ id: missionId, config, waypoints, pois });
124135
} catch (err: any) {
125136
console.error("KMZ import error:", err);
126-
res.status(500).json({ error: err.message || "Failed to parse KMZ" });
137+
res.status(500).json({ error: "Failed to parse KMZ" });
127138
}
128139
},
129140
);

packages/backend/src/services/authService.ts

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,28 @@
11
import bcrypt from "bcryptjs";
22
import jwt from "jsonwebtoken";
33

4-
const JWT_SECRET =
5-
process.env.JWT_SECRET || "genmap-dev-secret-change-in-production";
4+
function getJwtSecret(): string {
5+
const secret = process.env.JWT_SECRET;
6+
if (!secret) {
7+
const selfHosted = (process.env.SELF_HOSTED ?? "true") === "true";
8+
if (selfHosted) {
9+
// Self-hosted dev mode: use a default secret with a warning
10+
console.warn(
11+
"WARNING: JWT_SECRET is not set. Using insecure default. Set JWT_SECRET in production.",
12+
);
13+
return "droneroute-dev-secret-do-not-use-in-production";
14+
}
15+
throw new Error(
16+
"JWT_SECRET environment variable is required in cloud mode",
17+
);
18+
}
19+
if (secret.length < 32) {
20+
throw new Error("JWT_SECRET must be at least 32 characters for security");
21+
}
22+
return secret;
23+
}
24+
25+
const JWT_SECRET = getJwtSecret();
626
const TOKEN_EXPIRY = "7d";
727

828
export function hashPassword(password: string): string {
@@ -17,9 +37,14 @@ export function generateToken(userId: string, isAdmin: boolean): string {
1737
return jwt.sign({ userId, isAdmin }, JWT_SECRET, { expiresIn: TOKEN_EXPIRY });
1838
}
1939

20-
export function verifyToken(token: string): { userId: string; isAdmin: boolean } | null {
40+
export function verifyToken(
41+
token: string,
42+
): { userId: string; isAdmin: boolean } | null {
2143
try {
22-
return jwt.verify(token, JWT_SECRET) as { userId: string; isAdmin: boolean };
44+
return jwt.verify(token, JWT_SECRET) as {
45+
userId: string;
46+
isAdmin: boolean;
47+
};
2348
} catch {
2449
return null;
2550
}

packages/frontend/src/lib/api.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,15 @@ export const api = {
6363
// Admin API
6464
export const adminApi = {
6565
getUsers: (page = 1, perPage = 20) =>
66-
api.get<PaginatedResponse<AdminUser>>(`/admin/users?page=${page}&perPage=${perPage}`),
67-
banUser: (id: string) => api.post<{ message: string }>(`/admin/users/${id}/ban`),
68-
unbanUser: (id: string) => api.post<{ message: string }>(`/admin/users/${id}/unban`),
69-
promoteUser: (id: string) => api.post<{ message: string }>(`/admin/users/${id}/promote`),
70-
demoteUser: (id: string) => api.post<{ message: string }>(`/admin/users/${id}/demote`),
66+
api.get<PaginatedResponse<AdminUser>>(
67+
`/admin/users?page=${page}&perPage=${perPage}`,
68+
),
69+
banUser: (id: string) =>
70+
api.post<{ message: string }>(`/admin/users/${id}/ban`),
71+
unbanUser: (id: string) =>
72+
api.post<{ message: string }>(`/admin/users/${id}/unban`),
73+
promoteUser: (id: string) =>
74+
api.post<{ message: string }>(`/admin/users/${id}/promote`),
75+
demoteUser: (id: string) =>
76+
api.post<{ message: string }>(`/admin/users/${id}/demote`),
7177
};

0 commit comments

Comments
 (0)