Skip stale fs store artifacts
What the developer wrote
Authored by benthecarman
Skip stale fs store artifacts
The exhaustive filesystem store listing treated leftover temp and trash
files as namespace directories after identifying them as non-keys.
Skip those artifacts before recursing so migrations can ignore crash
leftovers.
What changed, and why it matters
This commit fixes a bug in the Lightning Dev Kit's filesystem storage module. Previously, leftover temporary and 'trash' files from crashes or interrupted operations were mistakenly treated as real data folders during listing and migration. The patch makes the code skip these stale artifacts, preventing potential confusion or errors during data migration. It is a defensive hardening fix rather than a directly exploitable vulnerability.
Apply the patch to ensure stale store artifacts are not misinterpreted during key listing and migration. Monitor for any related migration failures in environments with unclean shutdowns.
Security signals we found
Leftover temp/trash files incorrectly treated as namespace directories
Potential migration confusion from crash artifacts
Defensive filesystem traversal hardening
No direct memory safety or cryptographic issue in diff
Evidence from the diff
The change introduces a helper dir_entry_is_store_artifact() that identifies paths with .tmp or .trash extensions and skips them during FilesystemStoreState::read_all_keys() traversal at all three directory levels. It also refactors dir_entry_is_key() to use the same helper. A new unit test verifies that list_all_keys() ignores leftover .tmp and .trash files at the top, primary, and secondary levels. The Windows-specific cleanup of .trash files is preserved.
Changed components
lightning-persister/src/fs_store/common.rslightning-persister/src/fs_store/v1.rsInspect captured patch +48 / −12
diff --git a/lightning-persister/src/fs_store/common.rs b/lightning-persister/src/fs_store/common.rs
index 77321f6..6eaa0db 100644
--- a/lightning-persister/src/fs_store/common.rs
+++ b/lightning-persister/src/fs_store/common.rs
@@ -653,6 +653,9 @@ impl FilesystemStoreState {
'primary_loop: for primary_entry in fs::read_dir(prefixed_dest)? {
let primary_entry = primary_entry?;
let primary_path = primary_entry.path();
+ if dir_entry_is_store_artifact(&primary_path) {
+ continue 'primary_loop;
+ }
if dir_entry_is_key(&primary_entry)? {
let primary_namespace = String::new();
@@ -666,6 +669,9 @@ impl FilesystemStoreState {
'secondary_loop: for secondary_entry in fs::read_dir(&primary_path)? {
let secondary_entry = secondary_entry?;
let secondary_path = secondary_entry.path();
+ if dir_entry_is_store_artifact(&secondary_path) {
+ continue 'secondary_loop;
+ }
if dir_entry_is_key(&secondary_entry)? {
let primary_namespace = get_key_from_dir_entry_path(
@@ -683,6 +689,9 @@ impl FilesystemStoreState {
for tertiary_entry in fs::read_dir(&secondary_path)? {
let tertiary_entry = tertiary_entry?;
let tertiary_path = tertiary_entry.path();
+ if dir_entry_is_store_artifact(&tertiary_path) {
+ continue;
+ }
if dir_entry_is_key(&tertiary_entry)? {
let primary_namespace = get_key_from_dir_entry_path(
@@ -720,20 +729,25 @@ impl FilesystemStoreState {
}
}
+fn dir_entry_is_store_artifact(path: &Path) -> bool {
+ match path.extension().and_then(|ext| ext.to_str()) {
+ Some("tmp") => true,
+ Some("trash") => {
+ #[cfg(target_os = "windows")]
+ {
+ // Clean up any trash files lying around.
+ fs::remove_file(path).ok();
+ }
+ true
+ },
+ _ => false,
+ }
+}
+
pub(crate) fn dir_entry_is_key(dir_entry: &fs::DirEntry) -> Result<bool, lightning::io::Error> {
let p = dir_entry.path();
- if let Some(ext) = p.extension() {
- #[cfg(target_os = "windows")]
- {
- // Clean up any trash files lying around.
- if ext == "trash" {
- fs::remove_file(p).ok();
- return Ok(false);
- }
- }
- if ext == "tmp" {
- return Ok(false);
- }
+ if dir_entry_is_store_artifact(&p) {
+ return Ok(false);
}
let file_type = dir_entry.file_type()?;
diff --git a/lightning-persister/src/fs_store/v1.rs b/lightning-persister/src/fs_store/v1.rs
index 776aba6..7f47c59 100644
--- a/lightning-persister/src/fs_store/v1.rs
+++ b/lightning-persister/src/fs_store/v1.rs
@@ -186,6 +186,28 @@ mod tests {
assert_eq!(listed_keys.len(), 0);
}
+ #[test]
+ fn list_all_keys_skips_leftover_store_artifacts() {
+ let mut temp_path = std::env::temp_dir();
+ temp_path.push("test_list_all_keys_skips_leftover_store_artifacts");
+ let fs_store = FilesystemStore::new(temp_path.clone());
+ KVStoreSync::write(&fs_store, "primary", "secondary", "key", vec![1]).unwrap();
+
+ fs::write(temp_path.join("top_level.0.tmp"), b"stale").unwrap();
+ fs::write(temp_path.join("top_level.0.trash"), b"stale").unwrap();
+
+ let primary_path = temp_path.join("primary");
+ fs::write(primary_path.join("primary_level.0.tmp"), b"stale").unwrap();
+ fs::write(primary_path.join("primary_level.0.trash"), b"stale").unwrap();
+
+ let secondary_path = primary_path.join("secondary");
+ fs::write(secondary_path.join("secondary_level.0.tmp"), b"stale").unwrap();
+ fs::write(secondary_path.join("secondary_level.0.trash"), b"stale").unwrap();
+
+ let keys = fs_store.list_all_keys().unwrap();
+ assert_eq!(keys, vec![("primary".to_string(), "secondary".to_string(), "key".to_string())]);
+ }
+
#[test]
fn test_data_migration() {
let mut source_temp_path = std::env::temp_dir();
Why this scored 34/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.