Skip non-key entries in list_paginated
What the developer wrote
Authored by benthecarman
Skip non-key entries in list_paginated
Use dir_entry_is_key in list_paginated_impl to skip .tmp files,
directories, and other non-key entries, to be the same as list_impl.
Claude added a test for this as well.
What changed, and why it matters
This commit fixes a consistency bug in a file-based storage component used by Lightning Dev Kit. A paginated listing function was including temporary files and stray directories as if they were real data keys, which could confuse callers, leak internal filenames, or cause errors when reading back data. The fix makes the paginated listing skip the same non-key entries that the non-paginated listing already skipped.
Review whether any downstream code already received or acted on bogus keys from list_paginated; consider backporting to release branches that include FilesystemStoreV2. No immediate emergency response is indicated, but the fix should be included in the next maintenance release.
Security signals we found
Information disclosure: paginated listing could expose internal .tmp filenames and stray directory names to API consumers
Availability/reliability: callers iterating returned keys could fail when later reading entries that are not real persisted keys
Behavioral inconsistency between paginated and non-paginated listing APIs in the same store
Regression test added for non-key entry filtering
Evidence from the diff
In lightning-persister’s FilesystemStoreV2, list_paginated_impl previously iterated every fs::read_dir entry and passed each path to get_key_from_dir_entry_path without filtering. That meant .tmp files (inflight writes) and stray directories under a namespace could be returned as keys or trigger errors. The patch reuses the existing dir_entry_is_key helper (now crate-visible) to skip non-key entries, matching list_impl behavior. It also adds a regression test that writes real keys, drops a .tmp file and a stray directory on disk, and asserts the paginated listing returns only the two real keys.
Changed components
lightning-persister/src/fs_store/v2.rslightning-persister/src/fs_store/common.rsInspect captured patch +41 / −2
diff --git a/lightning-persister/src/fs_store/common.rs b/lightning-persister/src/fs_store/common.rs
index 7aef941..77321f6 100644
--- a/lightning-persister/src/fs_store/common.rs
+++ b/lightning-persister/src/fs_store/common.rs
@@ -720,7 +720,7 @@ impl FilesystemStoreState {
}
}
-fn dir_entry_is_key(dir_entry: &fs::DirEntry) -> Result<bool, lightning::io::Error> {
+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")]
diff --git a/lightning-persister/src/fs_store/v2.rs b/lightning-persister/src/fs_store/v2.rs
index 4260387..773b22a 100644
--- a/lightning-persister/src/fs_store/v2.rs
+++ b/lightning-persister/src/fs_store/v2.rs
@@ -1,5 +1,7 @@
//! Objects related to [`FilesystemStoreV2`] live here.
-use crate::fs_store::common::{get_key_from_dir_entry_path, FilesystemStoreState};
+use crate::fs_store::common::{
+ dir_entry_is_key, get_key_from_dir_entry_path, FilesystemStoreState,
+};
use lightning::util::persist::{
KVStoreSync, MigratableKVStore, PageToken, PaginatedKVStoreSync, PaginatedListResponse,
@@ -108,6 +110,16 @@ impl FilesystemStoreState {
for dir_entry in fs::read_dir(&prefixed_dest)? {
let dir_entry = dir_entry?;
+ match dir_entry_is_key(&dir_entry) {
+ // Entry is not a key (e.g., .tmp file, directory), skip it.
+ Ok(false) => continue,
+ // Entry is a valid key file, proceed to collect it.
+ Ok(true) => {},
+ // Entry may have been deleted between read_dir and our check. Include
+ // it anyway to give a more consistent view, matching list's behavior.
+ Err(_) => {},
+ }
+
let key =
get_key_from_dir_entry_path(&dir_entry.path(), prefixed_dest.as_path(), false)?;
// Get modification time as millis since epoch
@@ -616,6 +628,33 @@ mod tests {
assert_eq!(response.keys[3], "apple");
}
+ #[test]
+ fn test_paginated_listing_skips_tmp_files() {
+ use lightning::util::persist::{KVStoreSync, PaginatedKVStoreSync};
+
+ let mut temp_path = std::env::temp_dir();
+ temp_path.push("test_paginated_listing_skips_tmp_files_v2");
+ let fs_store = FilesystemStoreV2::new(temp_path.clone()).unwrap();
+
+ let data = vec![42u8; 32];
+
+ // Write some real keys
+ KVStoreSync::write(&fs_store, "ns", "sub", "key0", data.clone()).unwrap();
+ std::thread::sleep(std::time::Duration::from_millis(10));
+ KVStoreSync::write(&fs_store, "ns", "sub", "key1", data.clone()).unwrap();
+
+ // Create a .tmp file and a subdirectory directly on disk
+ let dir = temp_path.join("ns").join("sub");
+ fs::write(dir.join("inflight.tmp"), &data).unwrap();
+ fs::create_dir_all(dir.join("stray_dir")).unwrap();
+
+ // Paginated listing should only return the two real keys
+ let response = PaginatedKVStoreSync::list_paginated(&fs_store, "ns", "sub", None).unwrap();
+ assert_eq!(response.keys.len(), 2);
+ assert!(response.keys.contains(&"key0".to_string()));
+ assert!(response.keys.contains(&"key1".to_string()));
+ }
+
#[test]
fn test_rejects_v1_data_directory() {
let mut temp_path = std::env::temp_dir();
Why this scored 37/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.