From 8fc132eeadc278102845c29373731ab37a57bc2a Mon Sep 17 00:00:00 2001 From: Nick Walters Date: Tue, 6 Oct 2026 13:58:54 +0800 Subject: [PATCH] fix(posixfs): refuse paths outside the configured posixfs root `posixfs consistency` and `posixfs scan` take a path, find the storage it belongs to, and then build their ignore rules (and for scan, the driver) from the configured root. The ignore rules compare plain path strings. When the path is in another storage, reaches the storage through a symlink, or (for consistency) is relative, the internal directories .Trash, .oc-nodes and .oc-tmp are not recognised, and consistency writes node attributes onto them. Make consistency's paths absolute like scan already does, and check that every path is under the configured root as spelled in the configuration. --- opencloud/pkg/command/posixfs.go | 45 +++++++++++++++++++++- opencloud/pkg/command/posixfs_test.go | 55 +++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 opencloud/pkg/command/posixfs_test.go diff --git a/opencloud/pkg/command/posixfs.go b/opencloud/pkg/command/posixfs.go index 01bab8b75b..63ea146eaa 100644 --- a/opencloud/pkg/command/posixfs.go +++ b/opencloud/pkg/command/posixfs.go @@ -107,12 +107,16 @@ the command is aborted with an error before performing any scanning.`, path = v } } - // not ensuring whether the path is under the storage root here, will be done when iterating over them + // checkStorageRoot below makes sure the path is in the configured storage path = filepath.Clean(path) paths = append(paths, path) } } + if err := checkStorageRoot(storageRoot, paths); err != nil { + return err + } + var scan func(path string) error = nil { // We want to initialize the driver but disable scanfs on boot, so we can trigger it manually afterwards @@ -225,6 +229,16 @@ The provided arguments determines the scope of the check: if len(args) == 0 { args = []string{cfg.Drivers.Posix.Root} } + for i, arg := range args { + abs, err := filepath.Abs(arg) + if err != nil { + return fmt.Errorf("failed to make the path %q absolute: %w", arg, err) + } + args[i] = abs + } + if err := checkStorageRoot(cfg.Drivers.Posix.Root, args); err != nil { + return err + } log := logger("posixfs") recalculateChecksums, err := cmd.Flags().GetBool("fix-checksums") if err != nil { @@ -281,6 +295,35 @@ func findStorageRoot(path string) (string, error) { } } +// checkStorageRoot returns an error if a path is not inside the configured +// posixfs storage, spelled the way the configuration spells it. The commands +// build their ignore rules and the driver from the configured root, and those +// compare plain path strings. A path in another storage, or one reached through +// a symlink, would not have its internal directories (.Trash, .oc-nodes, +// .oc-tmp) recognised, and node attributes would be written onto them. Paths +// must be absolute. Paths outside any storage are left to the caller, which +// reports them. +func checkStorageRoot(configured string, paths []string) error { + if _, err := os.Stat(configured); err != nil { + return fmt.Errorf("the configured posixfs root '%s' is not accessible: %w", configured, err) + } + configured = filepath.Clean(configured) + for _, path := range paths { + if !filepath.IsAbs(path) { + return fmt.Errorf("'%s' is not an absolute path", path) + } + root, err := findStorageRoot(path) + if err != nil { + continue + } + if filepath.Clean(root) != configured { + return fmt.Errorf("'%s' is not under the configured posixfs root '%s' (it looks like part of a storage at '%s'). "+ + "Pass a path under '%s', or set STORAGE_USERS_POSIX_ROOT to the storage you mean", path, configured, root, configured) + } + } + return nil +} + // isSpaceRoot reports whether the given path is a space root, which is // identified by the presence of the space ID attribute. func isSpaceRoot(path string) bool { diff --git a/opencloud/pkg/command/posixfs_test.go b/opencloud/pkg/command/posixfs_test.go new file mode 100644 index 0000000000..780dd5ebb1 --- /dev/null +++ b/opencloud/pkg/command/posixfs_test.go @@ -0,0 +1,55 @@ +package command + +import ( + "os" + "path/filepath" + "testing" + + "github.com/test-go/testify/require" +) + +func newStorage(t *testing.T, root string) string { + t.Helper() + for _, dir := range []string{"indexes", "users/space1/docs"} { + require.NoError(t, os.MkdirAll(filepath.Join(root, dir), 0o700)) + } + return root +} + +func TestCheckStorageRoot(t *testing.T) { + configured := newStorage(t, t.TempDir()) + other := newStorage(t, t.TempDir()) + nested := newStorage(t, filepath.Join(configured, "users/space1/docs/nested")) + link := filepath.Join(t.TempDir(), "link") + require.NoError(t, os.Symlink(configured, link)) + + tests := []struct { + name string + configured string + paths []string + wantErr string + }{ + {"path in the configured storage", configured, []string{filepath.Join(configured, "users/space1/docs")}, ""}, + {"the configured root itself", configured, []string{configured}, ""}, + {"path outside any storage", configured, []string{t.TempDir()}, ""}, + {"path in another storage", configured, []string{filepath.Join(other, "users/space1")}, "not under the configured posixfs root"}, + {"a later path in another storage", configured, []string{configured, filepath.Join(other, "users")}, other}, + {"path in a storage nested inside the configured one", configured, []string{filepath.Join(nested, "users")}, nested}, + // the ignore rules compare strings, so a symlink in either path must be refused + {"configured root through a symlink", link, []string{filepath.Join(configured, "users/space1")}, "not under the configured posixfs root"}, + {"path through a symlink", configured, []string{filepath.Join(link, "users/space1")}, "not under the configured posixfs root"}, + {"relative path", configured, []string{"users/space1"}, "not an absolute path"}, + {"configured root that doesn't exist", filepath.Join(configured, "missing"), []string{configured}, "not accessible"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := checkStorageRoot(tc.configured, tc.paths) + if tc.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + require.Contains(t, err.Error(), tc.wantErr) + }) + } +}