From 1852ba439da2cf6c4200d2f170d81247099d15e7 Mon Sep 17 00:00:00 2001 From: dev9-bb Date: Tue, 20 Oct 2026 17:29:20 +0800 Subject: [PATCH] fix: reject mixed V2 storage and harden cross-volume staging --- README.testing.md | 18 +++ common/block-backend-fs.c | 2 +- common/cf-v2-storage.c | 50 +++++++- common/cf-v2-storage.h | 1 + .../objstore/cf_v2_route_wiring_test.go | 29 +++-- fileserver/objstore/objstore.go | 17 ++- server/gc/seafile-session.c | 6 + server/seafile-session.c | 11 ++ tests/local_storage/block.c | 2 + tests/local_storage/include/evutil.h | 1 + tests/local_storage/include/seafile-session.h | 7 ++ tests/local_storage/object.c | 3 +- tests/local_storage/run.sh | 9 ++ tests/local_storage/storage.c | 114 ++++++++++++++++++ 14 files changed, 252 insertions(+), 18 deletions(-) create mode 100644 tests/local_storage/include/seafile-session.h create mode 100644 tests/local_storage/storage.c diff --git a/README.testing.md b/README.testing.md index 61a226eb..6f96822c 100644 --- a/README.testing.md +++ b/README.testing.md @@ -215,3 +215,21 @@ observes actual SQL waits for both unbind/publication orderings, including no-op Barrier completion uses an explicitly test-only reconciliation proof; context/ACL/lifecycle/lock reconciliation is not claimed. Run without Python `-O`; disabling assertions is rejected before creating any test schemas. + +## Local Filesystem Publication + +```bash +bash tests/local_storage/run.sh +cd fileserver && go test ./objstore/... -count=1 -v +``` + +The C suite executes the native block/metadata backends for native and managed +paths, checking 0777 shard creation under umask 0022, hidden staging, cleanup and +unchanged object enumeration. It also executes the production V2 configuration +guard (before backend construction in server, GC and repair) for storage-class +and mixed-backend conflicts. These guard tests do not start full server/GC +processes or access a database. On Linux, the copy test uses distinct filesystems +(`/tmp` and `/dev/shm`), verifies the destination stays unpublished during copy, +and injects copy/fsync errors to require staging cleanup. Non-Linux runs report +that cross-filesystem part as SKIP. SQL routing and lifecycle are not covered by +this suite. S3 tests still require the explicit endpoint described earlier. diff --git a/common/block-backend-fs.c b/common/block-backend-fs.c index 9b8dcfe1..fabde1aa 100644 --- a/common/block-backend-fs.c +++ b/common/block-backend-fs.c @@ -514,7 +514,7 @@ open_tmp_file (BlockBackend *bend, if (!get_block_path (bend, basename, final_path, store_id, version)) return -1; dir = g_path_get_dirname (final_path); - if (g_mkdir_with_parents (dir, 0700) < 0) { + if (g_mkdir_with_parents (dir, 0777) < 0) { g_free (dir); return -1; } diff --git a/common/cf-v2-storage.c b/common/cf-v2-storage.c index ee8272fb..97d7ef05 100644 --- a/common/cf-v2-storage.c +++ b/common/cf-v2-storage.c @@ -19,6 +19,33 @@ cf_v2_storage_enabled (void) g_key_file_has_key (seaf->config, "cloudfile_v2", "auto_local_root", NULL); } +/* Check before constructing any backend, including offline GC sessions. */ +int +cf_v2_storage_validate_config (GKeyFile *config) +{ + const char *sections[] = { + "commit_object_backend", "fs_object_backend", "block_backend" + }; + guint i; + if (!g_key_file_has_key (config, "cloudfile_v2", "auto_local_root", NULL)) + return 0; + if (g_key_file_get_boolean (config, "storage", "enable_storage_classes", NULL)) { + seaf_warning ("V2 auto_local_root cannot coexist with [storage] enable_storage_classes.\n"); + return -1; + } + for (i = 0; i < G_N_ELEMENTS (sections); ++i) { + char *name = g_key_file_get_string (config, sections[i], "name", NULL); + gboolean invalid = name && *name && strcmp (name, "filesystem") != 0; + if (invalid) + seaf_warning ("V2 auto_local_root requires filesystem backend: [%s] name=%s.\n", + sections[i], name); + g_free (name); + if (invalid) + return -1; + } + return 0; +} + static gboolean real_directory (const char *path) { @@ -201,19 +228,36 @@ int cf_v2_storage_copy (const char *src, const char *dst) { int ret = link (src, dst), input = -1, output = -1; - char *temporary; + char *temporary, *directory, *name; if (ret == 0 || errno == EEXIST) return 0; if (errno != EXDEV) return -1; - temporary = g_strconcat (dst, ".XXXXXX", NULL); + directory = g_path_get_dirname (dst); + name = g_path_get_basename (dst); + temporary = g_strdup_printf ("%s/.cf-tmp-%s-XXXXXX", directory, name); + g_free (directory); + g_free (name); input = g_open (src, O_RDONLY, 0); if (input < 0) goto out; output = g_mkstemp (temporary); if (output < 0) goto out; - ret = copy_fd (input, output); + /* copy_fd() closes its input (and sometimes its output) internally. + * Keep descriptor ownership here so error cleanup cannot close a reused FD. */ + for (;;) { + char buffer[8192]; + ssize_t len = readn (input, buffer, sizeof(buffer)); + if (len < 0 || (len > 0 && writen (output, buffer, len) != len)) { + ret = -1; + break; + } + if (len == 0) { + ret = fsync (output); + break; + } + } if (close (output) < 0) ret = -1; output = -1; diff --git a/common/cf-v2-storage.h b/common/cf-v2-storage.h index 70b971b7..7acf020d 100644 --- a/common/cf-v2-storage.h +++ b/common/cf-v2-storage.h @@ -8,6 +8,7 @@ int cf_v2_storage_reserve (const char *repo_id); int cf_v2_storage_activate (const char *repo_id); int cf_v2_storage_retire (const char *repo_id); gboolean cf_v2_storage_enabled (void); +int cf_v2_storage_validate_config (GKeyFile *config); int cf_v2_storage_copy (const char *src, const char *dst); #endif diff --git a/fileserver/objstore/cf_v2_route_wiring_test.go b/fileserver/objstore/cf_v2_route_wiring_test.go index 72f0a137..58135856 100644 --- a/fileserver/objstore/cf_v2_route_wiring_test.go +++ b/fileserver/objstore/cf_v2_route_wiring_test.go @@ -58,15 +58,28 @@ func TestV2ObjectStoreSelectsManagedRoute(t *testing.T) { func TestV2RootRejectsMixedLegacyStorageBackends(t *testing.T) { root := t.TempDir() - for _, backendName := range []string{"s3", "multiple"} { - confDir := t.TempDir() - value := "[cloudfile_v2]\nauto_local_root=" + root + - "\n[block_backend]\nname=" + backendName + "\n" - if err := os.WriteFile(filepath.Join(confDir, "seafile.conf"), []byte(value), 0600); err != nil { - t.Fatal(err) + configurations := []string{"[storage]\nenable_storage_classes=true\n"} + for _, section := range []string{"commit_object_backend", "fs_object_backend", "block_backend"} { + for _, backendName := range []string{"s3", "multiple", "multi", "unknown"} { + configurations = append(configurations, "["+section+"]\nname="+backendName+"\n") } - if _, err := newBackend(confDir, t.TempDir(), "blocks"); err == nil { - t.Fatalf("mixed %s backend must fail rather than silently diverge", backendName) + } + for _, configuration := range configurations { + for _, kind := range []string{"commits", "fs", "blocks"} { + t.Run(kind+"/"+configuration, func(t *testing.T) { + confDir, dataDir := t.TempDir(), t.TempDir() + value := "[cloudfile_v2]\nauto_local_root=" + root + "\n" + configuration + if err := os.WriteFile(filepath.Join(confDir, "seafile.conf"), []byte(value), 0600); err != nil { + t.Fatal(err) + } + if _, err := newBackend(confDir, dataDir, kind); err == nil { + t.Fatal("mixed backend must fail before creating any native backend") + } + entries, err := os.ReadDir(dataDir) + if err != nil || len(entries) != 0 { + t.Fatalf("invalid configuration performed filesystem I/O: %v, %v", entries, err) + } + }) } } } diff --git a/fileserver/objstore/objstore.go b/fileserver/objstore/objstore.go index 2943dfda..274ca7ed 100644 --- a/fileserver/objstore/objstore.go +++ b/fileserver/objstore/objstore.go @@ -69,6 +69,18 @@ func newBackend(seafileConfPath, seafileDataDir, objType string) (storageBackend return nil, err } + if config.Section("cloudfile_v2").HasKey("auto_local_root") { + if config.Section("storage").Key("enable_storage_classes").MustBool(false) { + return nil, fmt.Errorf("V2 auto_local_root cannot coexist with [storage] enable_storage_classes") + } + for _, name := range []string{"commit_object_backend", "fs_object_backend", "block_backend"} { + backendName := config.Section(name).Key("name").String() + if backendName != "" && backendName != "filesystem" { + return nil, fmt.Errorf("V2 auto_local_root requires filesystem backend: [%s] name=%s", name, backendName) + } + } + } + sectionName := map[string]string{ "commits": "commit_object_backend", "fs": "fs_object_backend", @@ -81,11 +93,6 @@ func newBackend(seafileConfPath, seafileDataDir, objType string) (storageBackend if err != nil || section.Key("name").String() == "" || section.Key("name").String() == "filesystem" { return newRoutedFSBackend(configPath, seafileDataDir, objType) } - // Legacy multi/S3 storage cannot coexist with the V2 filesystem root: - // a mixed C/Go backend would otherwise publish objects in different places. - if config.Section("cloudfile_v2").HasKey("auto_local_root") { - return nil, fmt.Errorf("V2 auto_local_root requires filesystem object backend") - } if section.Key("name").String() != "s3" { if section.Key("name").String() == "multiple" { return newMultiBackend(config, configPath, objType) diff --git a/server/gc/seafile-session.c b/server/gc/seafile-session.c index c7cfde72..a1b3dbde 100644 --- a/server/gc/seafile-session.c +++ b/server/gc/seafile-session.c @@ -8,6 +8,7 @@ #include "seafile-session.h" #include "seaf-utils.h" +#include "cf-v2-storage.h" #include "log.h" @@ -62,6 +63,11 @@ seafile_session_new(const char *central_config_dir, goto onerror; } + if (cf_v2_storage_validate_config (config) < 0) { + g_key_file_free (config); + goto onerror; + } + session = g_new0(SeafileSession, 1); session->seaf_dir = abs_seafile_dir; session->ccnet_dir = abs_ccnet_dir; diff --git a/server/seafile-session.c b/server/seafile-session.c index 26696ff4..b32ace2a 100644 --- a/server/seafile-session.c +++ b/server/seafile-session.c @@ -27,6 +27,7 @@ #include "seaf-db.h" #include "seaf-utils.h" #include "cf-ext.h" +#include "cf-v2-storage.h" #include "log.h" @@ -132,6 +133,11 @@ load_config (SeafileSession *session, const char *config_file_path) goto out; } + if (cf_v2_storage_validate_config (config) < 0) { + g_key_file_free (config); + return -1; + } + session->config = config; session->cloud_mode = g_key_file_get_boolean (config, @@ -401,6 +407,11 @@ seafile_repair_session_new(const char *central_config_dir, session->seaf_dir = abs_seafile_dir; session->ccnet_dir = abs_ccnet_dir; session->tmp_file_dir = tmp_file_dir; + if (cf_v2_storage_validate_config (config) < 0) { + g_key_file_free (config); + goto onerror; + } + session->config = config; session->is_repair = TRUE; diff --git a/tests/local_storage/block.c b/tests/local_storage/block.c index 4bf8aa4c..0957f4a1 100644 --- a/tests/local_storage/block.c +++ b/tests/local_storage/block.c @@ -4,12 +4,14 @@ static const char *repo="11111111-1111-4111-8111-111111111111"; static const char *oid="abcdef0123456789012345678901234567890123"; static gboolean count(const char *r,int v,const char *id,void *p) {g_assert_cmpstr(id,==,oid);(*(int*)p)++;return TRUE;} int main(int argc,char **argv) { + umask(0022); g_assert_cmpint(argc,==,2);char *legacy=g_build_filename(argv[1],"tmpfiles",NULL); g_assert_true(g_file_set_contents(legacy,"poison",-1,NULL)); BlockBackend *b=block_backend_fs_new(argv[1],legacy);g_assert_nonnull(b); BHandle *h=b->open_block(b,repo,1,oid,BLOCK_WRITE);g_assert_nonnull(h); char final[SEAF_PATH_MAX];g_assert_nonnull(get_block_path(b,oid,final,repo,1)); char *parent=g_path_get_dirname(final),*actual=g_path_get_dirname(h->tmp_file); + struct stat shard_stat;g_assert_cmpint(stat(parent,&shard_stat),==,0);g_assert_cmpuint(shard_stat.st_mode & 0777,==,0755); g_assert_cmpstr(parent,==,actual);g_assert_false(g_file_test(final,G_FILE_TEST_EXISTS)); g_assert_cmpint(b->write_block(b,h,"complete",8),==,8); int n=0;g_assert_cmpint(b->foreach_block(b,repo,1,count,&n),==,0);g_assert_cmpint(n,==,0); diff --git a/tests/local_storage/include/evutil.h b/tests/local_storage/include/evutil.h index 144b3d5c..ee649705 100644 --- a/tests/local_storage/include/evutil.h +++ b/tests/local_storage/include/evutil.h @@ -4,6 +4,7 @@ #ifndef CF_LOCAL_STORAGE_TEST_EVUTIL_H #define CF_LOCAL_STORAGE_TEST_EVUTIL_H +#include typedef int evutil_socket_t; #endif diff --git a/tests/local_storage/include/seafile-session.h b/tests/local_storage/include/seafile-session.h new file mode 100644 index 00000000..d02e5699 --- /dev/null +++ b/tests/local_storage/include/seafile-session.h @@ -0,0 +1,7 @@ +/* Only the fields used by cf-v2-storage.c; no database is opened by these tests. */ +#include "seaf-db.h" +typedef struct { + GKeyFile *config; + SeafDB *db; +} SeafileSession; +extern SeafileSession *seaf; diff --git a/tests/local_storage/object.c b/tests/local_storage/object.c index fec6ae7b..2365c4c0 100644 --- a/tests/local_storage/object.c +++ b/tests/local_storage/object.c @@ -5,11 +5,12 @@ static const char *repo="11111111-1111-4111-8111-111111111111"; static const char *oid="abcdef0123456789012345678901234567890123"; static gboolean count(const char *r,int v,const char *id,void *p) {g_assert_cmpstr(id,==,oid);(*(int*)p)++;return TRUE;} int main(int argc,char **argv) { + umask(0022); g_assert_cmpint(argc,==,2);ObjBackend *b=obj_backend_fs_new(argv[1],"fs");g_assert_nonnull(b); g_assert_cmpint(b->write(b,repo,1,oid,"complete",8,TRUE),==,0); char final[SEAF_PATH_MAX];g_assert_cmpint(id_to_path(b->priv,oid,final,repo,1),==,0); fail_write=1;g_assert_cmpint(b->write(b,repo,1,oid,"partial",7,TRUE),==,-1);fail_write=0; - char *parent=g_path_get_dirname(final);GDir *dir=g_dir_open(parent,0,NULL);const char *name;int n=0; + char *parent=g_path_get_dirname(final);struct stat shard_stat;g_assert_cmpint(stat(parent,&shard_stat),==,0);g_assert_cmpuint(shard_stat.st_mode & 0777,==,0755);GDir *dir=g_dir_open(parent,0,NULL);const char *name;int n=0; while ((name=g_dir_read_name(dir))) {g_assert_false(g_str_has_prefix(name,".cf-tmp-"));n++;}g_dir_close(dir);g_assert_cmpint(n,==,1); char *orphan=g_build_filename(parent,".cf-tmp-crashed",NULL),*old=g_strconcat(final,".XXXXXX",NULL); g_assert_true(g_file_set_contents(orphan,"partial",-1,NULL));g_assert_true(g_file_set_contents(old,"partial",-1,NULL)); diff --git a/tests/local_storage/run.sh b/tests/local_storage/run.sh index e988706e..f03f06ec 100644 --- a/tests/local_storage/run.sh +++ b/tests/local_storage/run.sh @@ -20,3 +20,12 @@ for name in block object; do "$fixture/$name" "$fixture/$name-managed-data" done printf 'Native and managed C object staging, cleanup and enumeration passed\n' +# Unused routing/SQL functions are discarded; the production guard and copy +# implementation remain linked and executed without a database fixture. +link_flags=(-Wl,--gc-sections) +if [[ $(uname -s) == Darwin ]]; then link_flags=(-Wl,-dead_strip); fi +cc -D_GNU_SOURCE -ffunction-sections -fdata-sections \ + -I"$repo_root/tests/local_storage/include" -I"$repo_root/common" -I"$repo_root/lib" "${glib_cflags[@]}" \ + "$repo_root/tests/local_storage/storage.c" "${link_flags[@]}" "${glib_libs[@]}" -o "$fixture/storage" +mkdir "$fixture/copy-data" +"$fixture/storage" "$fixture/copy-data" diff --git a/tests/local_storage/storage.c b/tests/local_storage/storage.c new file mode 100644 index 00000000..e4cea514 --- /dev/null +++ b/tests/local_storage/storage.c @@ -0,0 +1,114 @@ +/* Execute the production config guard and copy path, with fault-only I/O seams. */ +#include "common.h" +#include +#include +#include +#include "utils.h" +static int fail_sync, fail_copy; +static const char *destination; +static int test_fsync (int fd) { + if (fail_sync) { errno = EIO; return -1; } + return fsync (fd); +} +#define fsync test_fsync +#include "../../common/cf-v2-storage.c" +#undef fsync +SeafileSession *seaf; + +ssize_t readn (int fd, void *buffer, size_t count) { return read (fd, buffer, count); } +ssize_t writen (int output, const void *buffer, size_t count) { + char *parent = g_path_get_dirname (destination); + GDir *dir = g_dir_open (parent, 0, NULL); + const char *name; + int staged = 0; + while ((name = g_dir_read_name (dir))) { + if (g_str_has_prefix (name, ".cf-tmp-object-")) + staged++; + g_assert_false (g_str_has_prefix (name, "object.")); + } + g_dir_close (dir); + g_free (parent); + g_assert_cmpint (staged, ==, 1); + g_assert_false (g_file_test (destination, G_FILE_TEST_EXISTS)); + ssize_t n = write (output, buffer, count); + if (fail_copy) { errno = EIO; return -1; } + return n; +} + +static void assert_clean (const char *directory) { + GDir *dir = g_dir_open (directory, 0, NULL); + const char *name; + while ((name = g_dir_read_name (dir))) + g_assert_false (g_str_has_prefix (name, ".cf-tmp-")); + g_dir_close (dir); +} + +static void check_config (const char *body, gboolean accepted) { + GKeyFile *config = g_key_file_new (); + g_assert_true (g_key_file_load_from_data (config, body, -1, G_KEY_FILE_NONE, NULL)); + g_assert_cmpint (cf_v2_storage_validate_config (config), ==, accepted ? 0 : -1); + g_key_file_free (config); +} + +int main (int argc, char **argv) { + const char *sections[] = {"commit_object_backend", "fs_object_backend", "block_backend"}; + const char *backends[] = {"s3", "multiple", "multi", "unknown"}; + g_assert_cmpint (argc, ==, 2); + check_config ("[storage]\nenable_storage_classes=true\n[block_backend]\nname=s3\n", TRUE); + check_config ("[cloudfile_v2]\nauto_local_root=/unused\n", TRUE); + check_config ("[cloudfile_v2]\nauto_local_root=\n[storage]\nenable_storage_classes=true\n", FALSE); + check_config ("[cloudfile_v2]\nauto_local_root=/unused\n[storage]\nenable_storage_classes=false\n", TRUE); + for (guint i = 0; i < G_N_ELEMENTS (sections); i++) { + char *config = g_strdup_printf ("[cloudfile_v2]\nauto_local_root=/unused\n[%s]\nname=filesystem\n", sections[i]); + check_config (config, TRUE); + g_free (config); + for (guint j = 0; j < G_N_ELEMENTS (backends); j++) { + config = g_strdup_printf ("[cloudfile_v2]\nauto_local_root=/unused\n[%s]\nname=%s\n", sections[i], backends[j]); + check_config (config, FALSE); + g_free (config); + } + } + char *src = g_build_filename (argv[1], "source", NULL); + char *same = g_build_filename (argv[1], "same-volume", NULL); + g_assert_true (g_file_set_contents (src, "complete payload", -1, NULL)); + g_assert_cmpint (cf_v2_storage_copy (src, same), ==, 0); + struct stat a, b; + g_assert_cmpint (stat (src, &a), ==, 0); + g_assert_cmpint (stat (same, &b), ==, 0); + g_assert_cmpuint (a.st_ino, ==, b.st_ino); +#ifdef __linux__ + /* tmpfs is an actual second filesystem, not a forced EXDEV return. */ + char *cross = g_strdup ("/dev/shm/cf-storage-copy-XXXXXX"); + g_assert_nonnull (g_mkdtemp (cross)); + g_assert_cmpint (stat (cross, &b), ==, 0); + g_assert_cmpuint (a.st_dev, !=, b.st_dev); + char *dst = g_build_filename (cross, "object", NULL); + destination = dst; + for (int fault = 0; fault < 3; fault++) { + fail_sync = fault == 1; + fail_copy = fault == 2; + g_assert_cmpint (cf_v2_storage_copy (src, dst), ==, fault ? -1 : 0); + if (!fault) { + char *actual = NULL; + g_assert_true (g_file_get_contents (dst, &actual, NULL, NULL)); + g_assert_cmpstr (actual, ==, "complete payload"); + g_free (actual); + g_unlink (dst); + } + g_assert_false (g_file_test (dst, G_FILE_TEST_EXISTS)); + assert_clean (cross); + } + fail_sync = fail_copy = 0; + g_assert_cmpint (cf_v2_storage_copy ("/does-not-exist", dst), ==, -1); + assert_clean (cross); + g_rmdir (cross); + g_free (cross); + g_free (dst); + g_print ("C configuration guard and real cross-filesystem copy/failure cleanup passed\n"); +#else + g_print ("C configuration guard and native hardlink passed; SKIP real cross-filesystem copy (Linux tmpfs required)\n"); +#endif + g_free (same); + g_free (src); + return 0; +}