From 4361cc3e839082c5d0cc6f70faf2271e42ed7a4a Mon Sep 17 00:00:00 2001 From: Seagulls Date: Sun, 11 Oct 2026 18:28:00 +0100 Subject: [PATCH 1/2] fix(cloud/local): propagate unreadable destination errors (#46 partial) --- docs/research/CLOUD-LOCAL-46-LISTING.md | 14 ++++++ internal/cloud/provider_local.go | 2 +- .../cloud/provider_local_integrity_test.go | 45 +++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 docs/research/CLOUD-LOCAL-46-LISTING.md create mode 100644 internal/cloud/provider_local_integrity_test.go diff --git a/docs/research/CLOUD-LOCAL-46-LISTING.md b/docs/research/CLOUD-LOCAL-46-LISTING.md new file mode 100644 index 00000000..d24dfc34 --- /dev/null +++ b/docs/research/CLOUD-LOCAL-46-LISTING.md @@ -0,0 +1,14 @@ +# #46: local-folder listing error — partial fix only + +- Upstream issue: https://github.com/Liquid-co/OpenSave/issues/46 +- Source research SHA: `e4c113ff3af3b9556444c1e511e15531f3b529c2` +- Pinned base `dev-public`: `7b690420c0fa03513764f4937af29bebf7f391a7` +- Branch is **fork-only**. It is not an accepted upstream implementation. + +## Change + +Return the actual `os.ReadDir` error from `localFolder.list()` instead of converting it to an empty successful listing. Tests cover absent, non-directory, and normally readable destinations. + +## Remaining risk + +`localFolder.put()` intentionally uses `writeFileWhole()`, which creates an absent destination path. An unmounted NAS can therefore still become a writable local fallback, even though the listing error is now surfaced when the configured directory is absent. A writable mountpoint underlying an unmounted share cannot be distinguished from the intended filesystem without a separate opt-in mount-identity contract. This is *not* a complete resolution of #46. diff --git a/internal/cloud/provider_local.go b/internal/cloud/provider_local.go index 37fad7b2..966cb651 100644 --- a/internal/cloud/provider_local.go +++ b/internal/cloud/provider_local.go @@ -41,7 +41,7 @@ func (p localFolder) list() ([]CloudFile, error) { } entries, err := os.ReadDir(p.cfg.URL) if err != nil { - return []CloudFile{}, nil + return nil, fmt.Errorf("read local folder destination %q: %w", p.cfg.URL, err) } var files []CloudFile for _, e := range entries { diff --git a/internal/cloud/provider_local_integrity_test.go b/internal/cloud/provider_local_integrity_test.go new file mode 100644 index 00000000..fc49e710 --- /dev/null +++ b/internal/cloud/provider_local_integrity_test.go @@ -0,0 +1,45 @@ +package cloud + +import ( + "os" + "path/filepath" + "testing" + + "github.com/opensave/opensave/internal/store" +) + +func TestLocalFolderListPropagatesDirectoryErrors(t *testing.T) { + t.Run("absent directory", func(t *testing.T) { + missing := filepath.Join(t.TempDir(), "missing-share") + files, err := (localFolder{cfg: store.CloudConfig{URL: missing}}).list() + if err == nil { + t.Fatalf("list = %+v, nil error; want missing-directory error", files) + } + }) + + t.Run("configured path is not a directory", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(path, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + files, err := (localFolder{cfg: store.CloudConfig{URL: path}}).list() + if err == nil { + t.Fatalf("list = %+v, nil error; want directory-read error", files) + } + }) +} + +func TestLocalFolderListStillSupportsOrdinaryFolders(t *testing.T) { + dir := t.TempDir() + want := []byte("synthetic zip") + if err := os.WriteFile(filepath.Join(dir, "game__main__snap_1.zip"), want, 0o600); err != nil { + t.Fatal(err) + } + files, err := (localFolder{cfg: store.CloudConfig{URL: dir}}).list() + if err != nil { + t.Fatalf("list: %v", err) + } + if len(files) != 1 || files[0].Name != "game__main__snap_1.zip" || files[0].SizeBytes != int64(len(want)) { + t.Fatalf("list = %+v", files) + } +} From a86d964eeb3e00f2959814e2e4973019bd9fd899 Mon Sep 17 00:00:00 2001 From: Seagulls Date: Thu, 15 Oct 2026 22:11:11 +0100 Subject: [PATCH 2/2] docs: remove internal research note from website tree --- docs/research/CLOUD-LOCAL-46-LISTING.md | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 docs/research/CLOUD-LOCAL-46-LISTING.md diff --git a/docs/research/CLOUD-LOCAL-46-LISTING.md b/docs/research/CLOUD-LOCAL-46-LISTING.md deleted file mode 100644 index d24dfc34..00000000 --- a/docs/research/CLOUD-LOCAL-46-LISTING.md +++ /dev/null @@ -1,14 +0,0 @@ -# #46: local-folder listing error — partial fix only - -- Upstream issue: https://github.com/Liquid-co/OpenSave/issues/46 -- Source research SHA: `e4c113ff3af3b9556444c1e511e15531f3b529c2` -- Pinned base `dev-public`: `7b690420c0fa03513764f4937af29bebf7f391a7` -- Branch is **fork-only**. It is not an accepted upstream implementation. - -## Change - -Return the actual `os.ReadDir` error from `localFolder.list()` instead of converting it to an empty successful listing. Tests cover absent, non-directory, and normally readable destinations. - -## Remaining risk - -`localFolder.put()` intentionally uses `writeFileWhole()`, which creates an absent destination path. An unmounted NAS can therefore still become a writable local fallback, even though the listing error is now surfaced when the configured directory is absent. A writable mountpoint underlying an unmounted share cannot be distinguished from the intended filesystem without a separate opt-in mount-identity contract. This is *not* a complete resolution of #46.