Detect nested v1 filesystem data
What the developer wrote
Authored by benthecarman
Detect nested v1 filesystem data
FilesystemStoreV2 already rejected v1 data when a key file was found
at the store root, but it did not inspect namespace directories. This
missed v1 layouts such as primary/key, where v2 expects primary to
contain secondary namespace directories.
For example, an ldk-node store can contain a BDK descriptor below a
namespace directory. The previous check would accept that directory as
v2 data because the root contained only directories, leaving the
incompatible descriptor file undetected.
What changed, and why it matters
This commit tightens a safety check in a new version of Lightning Dev Kit's file-based storage. The new store (v2) organizes data into folders-within-folders. Previously, it only rejected old v1 data if a stray file sat directly in the root folder, but it missed cases where an old-style file sat one folder deeper. That could let an incompatible v1 data file hide inside what looked like a valid v2 namespace folder, potentially causing confusion or data corruption when the v2 store later tries to use that folder as a namespace. The fix now scans one level deeper and refuses to open the store if it finds such files.
Treat as a defensive hardening fix. Users relying on FilesystemStoreV2 should upgrade so that legacy v1 files nested under namespace directories are correctly detected and rejected, preventing accidental operation on incompatible data layouts.
Security signals we found
Data-format compatibility guard strengthened
Previously undetected legacy data layout now rejected
Potential data corruption / misclassification risk from mixed v1/v2 layouts
No input validation bypass or memory-safety issue evident
Evidence from the diff
FilesystemStoreV2’s constructor now recursively inspects immediate child directories for files. In v1, keys were stored as files under namespace directories (e.g., primary/key or primary/secondary/key), whereas v2 expects primary/secondary/ to contain only further namespace directories. The previous top-level-only file check allowed a v1-style primary/key file to go undetected when the root contained only directories. The patch adds a second read_dir loop over each top-level directory, returning V1DataDetected for any file found one level down. Tests are expanded to cover top-level files, one-level-down files, valid empty directories, and valid v2 two-level namespace directories.
Changed components
lightning-persister/src/fs_store/v2.rsFilesystemStoreV2::new constructorFilesystemStoreV2Error::V1DataDetectedInspect captured patch +66 / −9
diff --git a/lightning-persister/src/fs_store/v2.rs b/lightning-persister/src/fs_store/v2.rs
index 2f79cae..6154d22 100644
--- a/lightning-persister/src/fs_store/v2.rs
+++ b/lightning-persister/src/fs_store/v2.rs
@@ -21,8 +21,8 @@ use std::sync::Arc;
/// An error returned when constructing a [`FilesystemStoreV2`].
#[derive(Debug)]
pub enum FilesystemStoreV2Error {
- /// The data directory contains a file at the top level, indicating it was previously used
- /// by [`FilesystemStore`] (v1). Contains the path of the offending file.
+ /// The data directory contains a file where v2 expects a namespace directory, indicating it
+ /// was previously used by [`FilesystemStore`] (v1). Contains the path of the offending file.
///
/// [`FilesystemStore`]: crate::fs_store::v1::FilesystemStore
V1DataDetected(PathBuf),
@@ -35,7 +35,7 @@ impl fmt::Display for FilesystemStoreV2Error {
match self {
Self::V1DataDetected(path) => write!(
f,
- "Found file `{}` in the top-level data directory. \
+ "Found file `{}` where FilesystemStoreV2 expects a namespace directory. \
This indicates the directory was previously used by FilesystemStore (v1). \
Please migrate your data or use a different directory.",
path.display()
@@ -97,18 +97,28 @@ impl FilesystemStoreV2 {
/// Constructs a new [`FilesystemStoreV2`].
///
/// Returns [`FilesystemStoreV2Error::V1DataDetected`] if the data directory already exists
- /// and contains files at the top level, which would indicate it was previously used by a
- /// [`FilesystemStore`] (v1). The v2 store expects only directories (namespaces) at the top
- /// level.
+ /// and contains files where v2 expects namespace directories, which would indicate it was
+ /// previously used by a [`FilesystemStore`] (v1). The v2 store expects only directories at
+ /// the top level and one level down.
///
/// [`FilesystemStore`]: crate::fs_store::v1::FilesystemStore
pub fn new(data_dir: PathBuf) -> Result<Self, FilesystemStoreV2Error> {
if data_dir.exists() {
for entry in fs::read_dir(&data_dir)? {
let entry = entry?;
- if entry.file_type()?.is_file() {
+ let file_type = entry.file_type()?;
+ if file_type.is_file() {
return Err(FilesystemStoreV2Error::V1DataDetected(entry.path()));
}
+
+ if file_type.is_dir() {
+ for child_entry in fs::read_dir(entry.path())? {
+ let child_entry = child_entry?;
+ if child_entry.file_type()?.is_file() {
+ return Err(FilesystemStoreV2Error::V1DataDetected(child_entry.path()));
+ }
+ }
+ }
}
}
@@ -699,6 +709,7 @@ mod tests {
fs::create_dir_all(&temp_path).unwrap();
// Create a file at the top level, as v1 would for an empty primary namespace
+ // and an empty secondary namespace.
fs::write(temp_path.join("some_key"), b"data").unwrap();
// V2 construction should fail
@@ -713,13 +724,59 @@ mod tests {
// Clean up
let _ = fs::remove_dir_all(&temp_path);
+ // Create a file one level down, as v1 would for a non-empty primary namespace
+ // and an empty secondary namespace.
+ fs::create_dir_all(temp_path.join("some_namespace")).unwrap();
+ fs::write(temp_path.join("some_namespace").join("some_key"), b"data").unwrap();
+
+ match FilesystemStoreV2::new(temp_path.clone()) {
+ Err(FilesystemStoreV2Error::V1DataDetected(path)) => {
+ assert_eq!(path, temp_path.join("some_namespace").join("some_key"));
+ },
+ Err(err) => panic!("Expected V1DataDetected, got {:?}", err),
+ Ok(_) => panic!("Expected error for directory with files one level down"),
+ }
+
+ let _ = fs::remove_dir_all(&temp_path);
+
+ // A v1 write with an empty primary namespace and non-empty secondary namespace
+ // is rejected by the KVStore API, but its filesystem layout would be the same
+ // one-level shape.
+ fs::create_dir_all(temp_path.join("some_secondary_namespace")).unwrap();
+ fs::write(temp_path.join("some_secondary_namespace").join("some_key"), b"data").unwrap();
+
+ match FilesystemStoreV2::new(temp_path.clone()) {
+ Err(FilesystemStoreV2Error::V1DataDetected(path)) => {
+ assert_eq!(path, temp_path.join("some_secondary_namespace").join("some_key"));
+ },
+ Err(err) => panic!("Expected V1DataDetected, got {:?}", err),
+ Ok(_) => panic!("Expected error for directory with files one level down"),
+ }
+
+ let _ = fs::remove_dir_all(&temp_path);
+
// An empty directory should succeed
fs::create_dir_all(&temp_path).unwrap();
let result = FilesystemStoreV2::new(temp_path.clone());
assert!(result.is_ok());
- // A directory with only subdirectories should succeed
- fs::create_dir_all(temp_path.join("some_namespace")).unwrap();
+ // A directory with only namespace subdirectories should succeed
+ fs::create_dir_all(temp_path.join("some_namespace").join("some_sub_namespace")).unwrap();
+ let result = FilesystemStoreV2::new(temp_path.clone());
+ assert!(result.is_ok());
+
+ // V1 data with non-empty primary and secondary namespaces has the same filesystem
+ // layout as valid v2 data, so construction must not reject this shape.
+ let fs_store = result.unwrap();
+ KVStoreSync::write(
+ &fs_store,
+ "some_namespace",
+ "some_sub_namespace",
+ "some_key",
+ b"data".to_vec(),
+ )
+ .unwrap();
+
let result = FilesystemStoreV2::new(temp_path);
assert!(result.is_ok());
}
Why this scored 26/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.