fix(security): harden update restore and notification CSRF

Protect backup restores and session-authenticated notification writes from unsafe state changes, while clearing CodeQL false positives without weakening intentional TLS pinning.

Refs CodeQL alerts #297-310

Thanks: INSOLVE (Honorary); Marco Jakobs (@jacotec); MyNameisStitch (@MyNameisStitch); Redspin (@playerumpknow)

.github/codeql/codeql-config.yml

@@ -63,6 +63,9 @@ query-filters:
       id: go/disabled-certificate-check
       paths:
         - betterdesk-agent/agent/agent.go
+        # API cert pinning uses VerifyPeerCertificate; development-only
+        # insecure TLS is additionally gated by !release and explicit opt-in.
+        - betterdesk-support-agent/apihttp.go
 
   # --- Test harness + global cookieParser (routes validate session/auth) ---
   - exclude:

web-nodejs/public/js/rdclient/filetransfer.js

@@ -134,7 +134,7 @@ class RDFileTransfer {
 
     static joinLocalPath(base, relativePath) {
         const rel = String(relativePath || '').replace(/\\/g, '/').replace(/^\/+/, '');
-        if (!base) return rel.replace(/\//g, '/');
+        if (!base) return rel;
         const sep = base.includes('\\') ? '\\' : '/';
         if (!rel) return base;
         const parts = rel.split('/').filter(Boolean);

web-nodejs/routes/bd-api.routes.js

@@ -38,6 +38,7 @@ const {
     requireDeviceToken,
     requireTokenDeviceMatch,
 } = require('../middleware/deviceAuth');
+const { doubleCsrfProtection } = require('../middleware/csrf');
 
 // ---------------------------------------------------------------------------
 //  Help requests & chat are stored on the Go server (single source of truth).
@@ -995,7 +996,7 @@ router.get('/notifications', requireAuth, async (req, res) => {
 //  POST /api/bd/notifications/:id/read — mark single notification read
 // ---------------------------------------------------------------------------
 
-router.post('/notifications/:id/read', requireAuth, async (req, res) => {
+router.post('/notifications/:id/read', doubleCsrfProtection, requireAuth, async (req, res) => {
     try {
         const userId = sessionUserId(req);
         if (!userId) {
@@ -1017,7 +1018,7 @@ router.post('/notifications/:id/read', requireAuth, async (req, res) => {
 //  POST /api/bd/notifications/read-all — mark all notifications read
 // ---------------------------------------------------------------------------
 
-router.post('/notifications/read-all', requireAuth, async (req, res) => {
+router.post('/notifications/read-all', doubleCsrfProtection, requireAuth, async (req, res) => {
     try {
         const userId = sessionUserId(req);
         if (!userId) {

web-nodejs/server.js

@@ -140,6 +140,9 @@ const sessionMiddleware = session({
     }
 });
 app.use(sessionMiddleware);
+// Generate the double-submit token before routes that need route-level CSRF
+// protection, including the session-authenticated notification endpoints.
+app.use(csrfTokenProvider);
 
 // Cache version — changes on every restart/deployment, stable during runtime.
 // Used in ?v= query strings so browsers cache assets per deployment.
@@ -230,7 +233,7 @@ app.use((req, res, next) => {
     next();
 });
 
-// CSRF protection — generate token for views, validate on POST/PUT/DELETE/PATCH.
+// CSRF protection — validate on POST/PUT/DELETE/PATCH.
 // Skip CSRF for device-facing API routes (/api/bd/*) — these MUST authenticate
 // via Bearer access token (session-cookie fallback is rejected in requireDeviceAuth).
 //
@@ -240,7 +243,6 @@ app.use((req, res, next) => {
 // non-browser HTTP client, so it is unsafe as a CSRF-bypass signal. Tauri
 // desktop clients receive the CSRF token via `csrfTokenProvider` and must
 // echo it back in the `X-CSRF-Token` header (csrf-csrf double-submit).
-app.use(csrfTokenProvider);
 app.use((req, res, next) => {
     if (req.path.startsWith('/api/bd/')) {
         return next();

web-nodejs/services/updateService.js

@@ -3553,6 +3553,9 @@ function restoreFromBackup(backupName) {
         const target = resolveManifestTarget(backupFilePath);
         const src = resolvePathUnderRoot(backupPath, backupFilePath);
         const dest = resolvePathUnderRoot(target.targetRoot, target.filePath);
+        if (isProtectedRuntimePath(dest)) {
+            throw new Error(`Refusing to restore protected runtime path: ${backupFilePath}`);
+        }
         if (fs.existsSync(src)) {
             fs.mkdirSync(path.dirname(dest), { recursive: true });
             fs.copyFileSync(src, dest);
@@ -3566,6 +3569,9 @@ function restoreFromBackup(backupName) {
         }
         const target = resolveManifestTarget(backupFilePath);
         const dest = resolvePathUnderRoot(target.targetRoot, target.filePath);
+        if (isProtectedRuntimePath(dest)) {
+            throw new Error(`Refusing to remove protected runtime path: ${backupFilePath}`);
+        }
         if (fs.existsSync(dest)) {
             fs.rmSync(dest, { force: true });
             removed++;

web-nodejs/services/userScopeService.js

@@ -109,11 +109,11 @@ async function warnUnknownPeerIds(db, peerIds) {
         try {
             const row = await lookup(peerId);
             if (!row) {
-                console.warn(`[userScope] peer grant references unknown device id: ${peerId}`);
+                console.warn('[userScope] peer grant references unknown device id:', peerId);
                 warned += 1;
             }
         } catch (err) {
-            console.warn(`[userScope] peer id lookup failed for ${peerId}:`, err.message);
+            console.warn('[userScope] peer id lookup failed for', peerId, err.message);
             warned += 1;
         }
     }

web-nodejs/tests/bd-api.routes.test.js

@@ -3,6 +3,8 @@
 const request = require('supertest');
 const express = require('express');
 const session = require('express-session');
+const cookieParser = require('cookie-parser');
+const { csrfTokenProvider } = require('../middleware/csrf');
 
 jest.mock('../services/authService', () => ({}));
 
@@ -83,21 +85,38 @@ describe('BD-API register rename guard', () => {
 
 describe('BD-API notification center', () => {
     let app;
+    const getCsrfCredentials = async () => {
+        const res = await request(app).get('/test-csrf');
+        const csrfCookie = (res.headers['set-cookie'] || [])
+            .find((cookie) => cookie.startsWith('__csrf'));
+        return {
+            token: res.body.token,
+            cookie: csrfCookie?.split(';', 1)[0],
+        };
+    };
 
     beforeEach(() => {
         jest.clearAllMocks();
         app = express();
         app.use(express.json());
+        app.use(cookieParser());
         app.use(session({
             secret: 'notification-test-secret',
             resave: false,
             saveUninitialized: true,
+            cookie: {
+                secure: true,
+                httpOnly: true,
+                sameSite: 'lax',
+            },
         }));
         app.use((req, _res, next) => {
             req.session.userId = 1;
             req.session.user = { id: 1, username: 'admin', role: 'admin' };
             next();
         });
+        app.use(csrfTokenProvider);
+        app.get('/test-csrf', (req, res) => res.json({ token: res.locals.csrfToken }));
         app.use('/api/bd', bdApiRoutes);
 
         db.getReadNotificationIds.mockResolvedValue(new Set());
@@ -148,16 +167,24 @@ describe('BD-API notification center', () => {
     it('does not expose registration notifications without enrollment permission', async () => {
         app = express();
         app.use(express.json());
+        app.use(cookieParser());
         app.use(session({
             secret: 'notification-test-secret',
             resave: false,
             saveUninitialized: true,
+            cookie: {
+                secure: true,
+                httpOnly: true,
+                sameSite: 'lax',
+            },
         }));
         app.use((req, _res, next) => {
             req.session.userId = 2;
             req.session.user = { id: 2, username: 'viewer', role: 'viewer' };
             next();
         });
+        app.use(csrfTokenProvider);
+        app.get('/test-csrf', (req, res) => res.json({ token: res.locals.csrfToken }));
         app.use('/api/bd', bdApiRoutes);
         db.getPendingRegistrations.mockResolvedValue([{
             id: 8,
@@ -173,6 +200,13 @@ describe('BD-API notification center', () => {
         expect(db.getPendingRegistrations).not.toHaveBeenCalled();
     });
 
+    it('rejects state changes without a CSRF token', async () => {
+        const res = await request(app).post('/api/bd/notifications/read-all');
+
+        expect(res.status).toBe(403);
+        expect(db.markAllNotificationsRead).not.toHaveBeenCalled();
+    });
+
     it('marks both help and registration notifications read', async () => {
         db.getPendingRegistrations.mockResolvedValue([{
             id: 9,
@@ -184,7 +218,11 @@ describe('BD-API notification center', () => {
             data: [{ id: 'help-1', created_at: '2026-08-23T11:00:00.000Z' }],
         });
 
-        const res = await request(app).post('/api/bd/notifications/read-all');
+        const csrf = await getCsrfCredentials();
+        const res = await request(app)
+            .post('/api/bd/notifications/read-all')
+            .set('Cookie', csrf.cookie)
+            .set('X-CSRF-Token', csrf.token);
 
         expect(res.status).toBe(200);
         expect(db.markAllNotificationsRead).toHaveBeenCalledWith(1, [

web-nodejs/tests/filetransfer.logic.test.js

@@ -46,6 +46,7 @@ describe('RDFileTransfer static helpers', () => {
         expect(RDFileTransfer.joinLocalPath('/home/me/dl', 'proj/readme.txt'))
             .toBe('/home/me/dl/proj/readme.txt');
         expect(RDFileTransfer.joinLocalPath('/tmp/out', '')).toBe('/tmp/out');
+        expect(RDFileTransfer.joinLocalPath('', 'proj/readme.txt')).toBe('proj/readme.txt');
     });
 });
 

web-nodejs/tests/updateService.consoleSync.test.js

@@ -3,6 +3,7 @@
 const fs = require('fs');
 const os = require('os');
 const path = require('path');
+const config = require('../config/config');
 const { createConsoleDeployGraph } = require('../lib/consoleDeployGraph');
 const {
     GITHUB_COMPARE_FILE_LIMIT,
@@ -11,6 +12,7 @@ const {
     getDownloadRetryDelayMs,
     ensureGoServerSignalRelayPorts,
     restoreServerBinaryBackup,
+    restoreFromBackup,
 } = require('../services/updateService');
 
 describe('updateService console sync helpers', () => {
@@ -107,4 +109,55 @@ describe('updateService console sync helpers', () => {
             error: 'Server binary backup path failed validation',
         });
     });
+
+    test('rejects traversal paths from a backup manifest', () => {
+        const backupRoot = path.join(config.dataDir, 'backups');
+        const backupName = `pre-update-${Date.now()}-${process.pid}`;
+        const backupPath = path.join(backupRoot, backupName);
+        fs.mkdirSync(backupPath, { recursive: true });
+        fs.writeFileSync(path.join(backupPath, 'manifest.json'), JSON.stringify({
+            files: ['../outside.txt'],
+        }));
+
+        try {
+            expect(() => restoreFromBackup(backupName))
+                .toThrow('Invalid path in backup manifest');
+        } finally {
+            fs.rmSync(backupPath, { recursive: true, force: true });
+        }
+    });
+
+    test('refuses to restore protected runtime files from a backup manifest', () => {
+        const backupRoot = path.join(config.dataDir, 'backups');
+        const backupName = `pre-update-${Date.now()}-${process.pid}`;
+        const backupPath = path.join(backupRoot, backupName);
+        fs.mkdirSync(backupPath, { recursive: true });
+        fs.writeFileSync(path.join(backupPath, 'manifest.json'), JSON.stringify({
+            files: ['console/.env'],
+        }));
+
+        try {
+            expect(() => restoreFromBackup(backupName))
+                .toThrow('Refusing to restore protected runtime path');
+        } finally {
+            fs.rmSync(backupPath, { recursive: true, force: true });
+        }
+    });
+
+    test('refuses to remove protected runtime files from a backup manifest', () => {
+        const backupRoot = path.join(config.dataDir, 'backups');
+        const backupName = `pre-update-${Date.now()}-${process.pid}`;
+        const backupPath = path.join(backupRoot, backupName);
+        fs.mkdirSync(backupPath, { recursive: true });
+        fs.writeFileSync(path.join(backupPath, 'manifest.json'), JSON.stringify({
+            removeOnRestore: ['console/.env'],
+        }));
+
+        try {
+            expect(() => restoreFromBackup(backupName))
+                .toThrow('Refusing to remove protected runtime path');
+        } finally {
+            fs.rmSync(backupPath, { recursive: true, force: true });
+        }
+    });
 });

web-nodejs/tests/userScopeService.test.js

@@ -41,7 +41,10 @@ describe('userScopeService peer grants', () => {
         };
 
         await expect(userScopeService.warnUnknownPeerIds(db, ['1', 'missing'])).resolves.toBeUndefined();
-        expect(warn).toHaveBeenCalledWith(expect.stringContaining('unknown device id: missing'));
+        expect(warn).toHaveBeenCalledWith(
+            '[userScope] peer grant references unknown device id:',
+            'missing'
+        );
         warn.mockRestore();
     });
 });