avoid deleting the backups of same-prefixed wallets
What the developer wrote
Authored by Craig Raw
avoid deleting the backups of same-prefixed wallets
What changed, and why it matters
This commit fixes a bug in how Sparrow Wallet finds and deletes old wallet backups. Previously, the backup cleanup logic used loose pattern matching that could accidentally treat backups of one wallet as if they belonged to another wallet with a similar name. For example, a wallet named 'Savings' might incorrectly match backup files for 'SavingsX' or 'Savings.old'. The result could be that backups of a different wallet get deleted, or that cleanup of the intended wallet's backups fails. The patch replaces the loose prefix-and-date check with a strict regular expression that requires an exact wallet name, a complete 14-digit timestamp, and a matching file extension. New unit tests confirm that only correctly named backups are selected.
Users should upgrade to a Sparrow Wallet release that includes this commit so that automatic backup retention does not accidentally delete backups of similarly named wallets. Developers should review whether any other file-selection routines in the codebase rely on prefix-only matching and consider applying the same strict whole-name pattern approach.
Security signals we found
Incorrect backup selection could lead to deletion of another wallet's backup files
Loose filename prefix matching allowed same-prefixed wallet names to collide
Date extraction regex accepted extra trailing characters, broadening matches
Fix uses strict whole-filename regex with quoted wallet name and fixed-width timestamp
New unit tests cover name collision, extension matching, and regex literalization
Evidence from the diff
Storage.getBackups previously listed backup files by checking name.startsWith(prefix_walletName-) and then extracting a date via a regex that tolerated extra characters after the timestamp. It also relied on getBackupDate returning non-null. This allowed filenames such as ‘SavingsX-20250101120000.mv.db’ or ‘Savings.old-20250101120000.mv.db’ to be associated with wallet ‘Savings’, because the prefix test did not enforce a boundary after walletName and the date regex ignored trailing text. The patch removes hasStartedSince and getBackupDate, and instead compiles a strict anchored-ish pattern: quote(prefix_walletName-) + 14 digits + quote(extension), then matches the whole filename. Sorting now uses the full filename in reverse lexicographic order, which is equivalent to reverse chronological order because the timestamp is fixed-width. Tests verify that longer wallet names, mismatched extensions, partial or over-long timestamps, and regex-special characters in names are handled correctly.
Changed components
com.sparrowwallet.sparrow.io.StorageWallet backup deletion and enumeration logicSparrow Wallet backup directory handlingInspect captured patch +55 / −30
diff --git a/src/main/java/com/sparrowwallet/sparrow/io/Storage.java b/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
index bae64cf..27e0f1d 100644
--- a/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
+++ b/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
@@ -26,13 +26,11 @@ import java.nio.file.attribute.PosixFilePermissions;
import java.security.cert.Certificate;
import java.security.cert.CertificateEncodingException;
import java.time.LocalDateTime;
-import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
-import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
@@ -41,7 +39,6 @@ public class Storage {
public static final ECKey NO_PASSWORD_KEY = ECKey.fromPublicOnly(ECKey.fromPrivate(Utils.hexToBytes("885e5a09708a167ea356a252387aa7c4893d138d632e296df8fbf5c12798bd28")));
private static final DateTimeFormatter BACKUP_DATE_FORMAT = DateTimeFormatter.ofPattern("yyyyMMddHHmmss");
- private static final Pattern DATE_PATTERN = Pattern.compile(".+-([0-9]{14}?).*");
private static final Set<String> warnedDirectories = ConcurrentHashMap.newKeySet();
public static final String WALLETS_DIR = "wallets";
@@ -274,17 +271,6 @@ public class Storage {
deleteBackups(null);
}
- private boolean hasStartedSince(File lastBackup) {
- try {
- LocalDateTime date = LocalDateTime.parse(getBackupDate(lastBackup.getName()), BACKUP_DATE_FORMAT);
- ProcessHandle.Info processInfo = ProcessHandle.current().info();
- return (processInfo.startInstant().isPresent() && processInfo.startInstant().get().isAfter(date.atZone(ZoneId.systemDefault()).toInstant()));
- } catch(Exception e) {
- log.error("Error parsing date for backup file " + lastBackup.getName(), e);
- return false;
- }
- }
-
private void deleteBackups(String prefix) {
File[] backups = getBackups(prefix);
for(File backup : backups) {
@@ -293,30 +279,21 @@ public class Storage {
}
File[] getBackups(String prefix) {
- File backupDir = getWalletsBackupDir();
+ return getBackups(getWalletsBackupDir(), prefix);
+ }
+
+ File[] getBackups(File backupDir, String prefix) {
String walletName = persistence.getWalletName(walletFile, null);
String extension = walletFile.getName().substring(walletName.length());
- File[] backups = backupDir.listFiles((dir, name) -> {
- return name.startsWith((prefix == null ? "" : prefix + "_") + walletName + "-") &&
- getBackupDate(name) != null &&
- (extension.isEmpty() || name.endsWith(extension));
- });
+ Pattern backupPattern = Pattern.compile(Pattern.quote((prefix == null ? "" : prefix + "_") + walletName + "-") + "[0-9]{14}" + Pattern.quote(extension));
+ File[] backups = backupDir.listFiles((dir, name) -> backupPattern.matcher(name).matches());
backups = backups == null ? new File[0] : backups;
- Arrays.sort(backups, Comparator.comparing(o -> getBackupDate(((File)o).getName())).reversed());
+ Arrays.sort(backups, Comparator.comparing(File::getName).reversed());
return backups;
}
- private String getBackupDate(String backupFileName) {
- Matcher matcher = DATE_PATTERN.matcher(backupFileName);
- if(matcher.matches()) {
- return matcher.group(1);
- }
-
- return null;
- }
-
private WalletAndKey migrateToDb(WalletAndKey masterWalletAndKey) throws IOException, StorageException {
if(getType() == PersistenceType.JSON) {
log.info("Migrating " + masterWalletAndKey.getWallet().getName() + " from JSON to DB persistence");
diff --git a/src/test/java/com/sparrowwallet/sparrow/io/StorageTest.java b/src/test/java/com/sparrowwallet/sparrow/io/StorageTest.java
index 1361a1d..06184e1 100644
--- a/src/test/java/com/sparrowwallet/sparrow/io/StorageTest.java
+++ b/src/test/java/com/sparrowwallet/sparrow/io/StorageTest.java
@@ -12,6 +12,9 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import java.io.*;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
public class StorageTest extends IoTest {
@Test
@@ -84,6 +87,51 @@ public class StorageTest extends IoTest {
Assertions.assertTrue(wallet.isValid());
}
+ @Test
+ public void getBackupsExcludesLongerWalletNames() throws IOException {
+ File backupDir = createBackupDir("Savings-20250101120000.mv.db", "Savings-20240101120000.mv.db",
+ "Savings-2023-20250101120000.mv.db", "Savings.old-20250101120000.mv.db", "SavingsX-20250101120000.mv.db");
+
+ assertBackups(backupDir, PersistenceType.DB, "Savings.mv.db", "Savings-20250101120000.mv.db", "Savings-20240101120000.mv.db");
+ assertBackups(backupDir, PersistenceType.DB, "Savings-2023.mv.db", "Savings-2023-20250101120000.mv.db");
+ assertBackups(backupDir, PersistenceType.DB, "Savings.old.mv.db", "Savings.old-20250101120000.mv.db");
+ }
+
+ @Test
+ public void getBackupsRequiresAWholeDateAndAMatchingExtension() throws IOException {
+ File backupDir = createBackupDir("Savings-20250101120000.mv.db", "Savings-2025010112000.mv.db", "Savings-202501011200000.mv.db",
+ "Savings-20250101120000.json", "Savings-20250101120000", "Savings.mv.db", "Savings-notes.txt");
+
+ assertBackups(backupDir, PersistenceType.DB, "Savings.mv.db", "Savings-20250101120000.mv.db");
+ assertBackups(backupDir, PersistenceType.JSON, "Savings.json", "Savings-20250101120000.json");
+ assertBackups(backupDir, PersistenceType.JSON, "Savings", "Savings-20250101120000");
+ }
+
+ @Test
+ public void getBackupsTreatsAWalletNameLiterally() throws IOException {
+ File backupDir = createBackupDir("SavingsXold-20250101120000.mv.db");
+
+ assertBackups(backupDir, PersistenceType.DB, "Savings.old.mv.db");
+ }
+
+ private File createBackupDir(String... backupNames) throws IOException {
+ Path backupDir = Files.createTempDirectory("sprw-backup");
+ backupDir.toFile().deleteOnExit();
+ for(String backupName : backupNames) {
+ File backup = backupDir.resolve(backupName).toFile();
+ backup.createNewFile();
+ backup.deleteOnExit();
+ }
+
+ return backupDir.toFile();
+ }
+
+ private void assertBackups(File backupDir, PersistenceType persistenceType, String walletFileName, String... expectedBackupNames) {
+ Storage storage = new Storage(persistenceType, new File(backupDir.getParentFile(), walletFileName));
+ File[] backups = storage.getBackups(backupDir, null);
+ Assertions.assertArrayEquals(expectedBackupNames, Arrays.stream(backups).map(File::getName).toArray(String[]::new));
+ }
+
@AfterEach
void tearDown() {
System.setProperty(Wallet.ALLOW_DERIVATIONS_MATCHING_OTHER_NETWORKS_PROPERTY, "false");
Why this scored 48/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.