From 0b226b873ffca0f0c33fea594e08a699a4e0a437 Mon Sep 17 00:00:00 2001
From: Travis Lesicka <TravisLesicka@gmail.com>
Date: Fri, 25 Oct 2024 15:22:07 +1000
Subject: [PATCH] Fixed issues recommended by coderabbitai

---
 .../src/components/modals/LoadBackupModal.tsx |  9 +++-
 .../modals/manager/DuplicateFileModal.tsx     | 42 ++++++++++++-------
 packages/loot-core/src/server/backups.web.ts  | 17 ++++++--
 packages/loot-core/src/server/main.ts         |  6 +++
 4 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/packages/desktop-client/src/components/modals/LoadBackupModal.tsx b/packages/desktop-client/src/components/modals/LoadBackupModal.tsx
index a4b841bf5d..3b9c875a5a 100644
--- a/packages/desktop-client/src/components/modals/LoadBackupModal.tsx
+++ b/packages/desktop-client/src/components/modals/LoadBackupModal.tsx
@@ -147,8 +147,13 @@ export function LoadBackupModal({
                     isLoading={loading === 'backup'}
                     onPress={async () => {
                       setLoading('backup');
-                      await dispatch(makeBackup());
-                      setLoading(null);
+                      try {
+                        await dispatch(makeBackup());
+                      } catch (error) {
+                        console.error('Failed to create backup:', error);
+                      } finally {
+                        setLoading(null);
+                      }
                     }}
                   >
                     <Trans>Backup now</Trans>
diff --git a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx
index 4f5e52c855..576dc06a65 100644
--- a/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx
+++ b/packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx
@@ -54,6 +54,9 @@ export function DuplicateFileModal({
     if (trimmedName.length > 100) {
       return t('Budget name is too long (max length 100)');
     }
+    if (!/^[a-zA-Z0-9 .\-_()]+$/.test(trimmedName)) {
+      return t('Budget name contains invalid characters');
+    }
     // Additional validation checks can go here
 
     return null;
@@ -75,22 +78,29 @@ export function DuplicateFileModal({
     if (!error) {
       setLoadingState(sync === 'cloudSync' ? 'cloud' : 'local');
 
-      await dispatch(
-        duplicateBudget({
-          id: 'id' in file ? file.id : undefined,
-          cloudId:
-            sync === 'cloudSync' && 'cloudFileId' in file
-              ? file.cloudFileId
-              : undefined,
-          oldName: file.name,
-          newName,
-          cloudSync: sync === 'cloudSync',
-          managePage,
-          loadBudget,
-        }),
-      );
-
-      setLoadingState(null);
+      try {
+        await dispatch(
+          duplicateBudget({
+            id: 'id' in file ? file.id : undefined,
+            cloudId:
+              sync === 'cloudSync' && 'cloudFileId' in file
+                ? file.cloudFileId
+                : undefined,
+            oldName: file.name,
+            newName,
+            cloudSync: sync === 'cloudSync',
+            managePage,
+            loadBudget,
+          }),
+        );
+      } catch (e) {
+        const newError =  new Error('Failed to duplicate budget');
+        if (onComplete) onComplete({ status: 'failed', error: newError });
+        else console.error('Failed to duplicate budget:', e);
+      } finally {
+        setLoadingState(null);
+      }
+
       if (onComplete) onComplete({ status: 'success' });
     } else {
       const failError = new Error(error);
diff --git a/packages/loot-core/src/server/backups.web.ts b/packages/loot-core/src/server/backups.web.ts
index 902ef0a1b8..dfda16351e 100644
--- a/packages/loot-core/src/server/backups.web.ts
+++ b/packages/loot-core/src/server/backups.web.ts
@@ -1,4 +1,5 @@
 // @ts-strict-ignore
+import { type Database } from '@jlongster/sql.js';
 import * as dateFns from 'date-fns';
 
 import * as connection from '../platform/server/connection';
@@ -116,10 +117,18 @@ export async function makeBackup(id: string) {
   );
 
   // Remove all the messages from the backup
-  const db = await sqlite.openDatabase(fs.join(budgetDir, backupId));
-  await sqlite.runQuery(db, 'DELETE FROM messages_crdt');
-  await sqlite.runQuery(db, 'DELETE FROM messages_clock');
-  sqlite.closeDatabase(db);
+  let db: Database;
+  try {
+    db = await sqlite.openDatabase(fs.join(budgetDir, backupId));
+    await sqlite.runQuery(db, 'DELETE FROM messages_crdt');
+    await sqlite.runQuery(db, 'DELETE FROM messages_clock');
+  } catch (error) {
+    console.error('Error cleaning up backup messages:', error);
+  } finally {
+    if (db) {
+      sqlite.closeDatabase(db);
+    }
+  }
 
   const toRemove = await updateBackups(await getBackups(id));
   for (const id of toRemove) {
diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts
index 6a27e0db7d..a595db0f60 100644
--- a/packages/loot-core/src/server/main.ts
+++ b/packages/loot-core/src/server/main.ts
@@ -1820,6 +1820,12 @@ handlers['duplicate-budget'] = async function ({
   open,
 }): Promise<string> {
   if (!id) throw new Error('Unable to duplicate a budget that is not local.');
+  if (!newName?.trim()) {
+    throw new Error('Budget name is required and cannot be empty');
+  }
+  if (!/^[a-zA-Z0-9 .\-_()]+$/.test(newName)) {
+    throw new Error('Budget name contains invalid characters');
+  }
 
   const budgetDir = fs.getBudgetDir(id);