Merge branch 'dane/fs-tests' into develop

go.mod

@@ -24,6 +24,7 @@ require (
 	github.com/docker/go-metrics v0.0.1 // indirect
 	github.com/docker/go-units v0.4.0 // indirect
 	github.com/fatih/color v1.9.0
+	github.com/franela/goblin v0.0.0-20200825194134-80c0062ed6cd
 	github.com/frankban/quicktest v1.10.2 // indirect
 	github.com/fsnotify/fsnotify v1.4.9 // indirect
 	github.com/gabriel-vasile/mimetype v1.1.1

go.sum

@@ -123,6 +123,8 @@ github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5Kwzbycv
 github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
 github.com/franela/goblin v0.0.0-20200105215937-c9ffbefa60db/go.mod h1:7dvUGVsVBjqR7JHJk0brhHOZYGmfBYOrK0ZhYMEtBr4=
+github.com/franela/goblin v0.0.0-20200825194134-80c0062ed6cd h1:b/30UOB56Rhfe185ZfgvZT0/HOql0OzxuiNOxRKXRXc=
+github.com/franela/goblin v0.0.0-20200825194134-80c0062ed6cd/go.mod h1:VzmDKDJVZI3aJmnRI9VjAn9nJ8qPPsN1fqzr9dqInIo=
 github.com/franela/goreq v0.0.0-20171204163338-bcd34c9993f8/go.mod h1:ZhphrRTfi2rbfLwlschooIH4+wKKDR4Pdxhh+TRoA20=
 github.com/frankban/quicktest v1.10.2 h1:19ARM85nVi4xH7xPXuc5eM/udya5ieh7b/Sv+d844Tk=
 github.com/frankban/quicktest v1.10.2/go.mod h1:K+q6oSqb0W0Ininfk863uOk1lMy69l/P6txr3mVT54s=

router/router_server_files.go

@@ -1,7 +1,6 @@
 package router
 
 import (
-	"bufio"
 	"context"
 	"github.com/gin-gonic/gin"
 	"github.com/pkg/errors"
@@ -30,34 +29,12 @@ func getServerFileContents(c *gin.Context) {
 	}
 	p = "/" + strings.TrimLeft(p, "/")
 
-	cleaned, err := s.Filesystem().SafePath(p)
+	st, err := s.Filesystem().Stat(p)
 	if err != nil {
-		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
+		TrackedServerError(err, s).AbortFilesystemError(c)
-			"error": "The file requested could not be found.",
-		})
 		return
 	}
 
-	st, err := s.Filesystem().Stat(cleaned)
-	if err != nil {
-		TrackedServerError(err, s).AbortWithServerError(c)
-		return
-	}
-
-	if st.Info.IsDir() {
-		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
-			"error": "The requested resource was not found on the system.",
-		})
-		return
-	}
-
-	f, err := os.Open(cleaned)
-	if err != nil {
-		TrackedServerError(err, s).AbortWithServerError(c)
-		return
-	}
-	defer f.Close()
-
 	c.Header("X-Mime-Type", st.Mimetype)
 	c.Header("Content-Length", strconv.Itoa(int(st.Info.Size())))
 
@@ -68,7 +45,10 @@ func getServerFileContents(c *gin.Context) {
 		c.Header("Content-Type", "application/octet-stream")
 	}
 
-	bufio.NewReader(f).WriteTo(c.Writer)
+	if err := s.Filesystem().Readfile(p, c.Writer); err != nil {
+		TrackedServerError(err, s).AbortFilesystemError(c)
+		return
+	}
 }
 
 // Returns the contents of a directory for a server.

server/archiver.go

@@ -62,10 +62,21 @@ func (a *Archiver) Archive() error {
 	}
 
 	for _, file := range fileInfo {
-		f, err := a.Server.Filesystem().SafeJoin(path, file)
+		f := filepath.Join(path, file.Name())
+		// If the file is a symlink we cannot safely assume that the result of a filepath.Join() will be
+		// a safe destination. We need to check if the file is a symlink, and if so pass off to the SafePath
+		// function to resolve it to the final destination.
+		//
+		// ioutil.ReadDir() calls Lstat, so this will work correctly. If it did not call Lstat, but rather
+		// just did a normal Stat call, this would fail since that would be looking at the symlink destination
+		// and not the actual file in this listing.
+		if file.Mode()&os.ModeSymlink != 0 {
+			f, err = a.Server.Filesystem().SafePath(filepath.Join(path, file.Name()))
+
 			if err != nil {
 				return err
 			}
+		}
 
 		files = append(files, f)
 	}

server/filesystem/disk_space.go

@@ -199,7 +199,18 @@ func (fs *Filesystem) hasSpaceFor(size int64) error {
 
 // Updates the disk usage for the Filesystem instance.
 func (fs *Filesystem) addDisk(i int64) int64 {
-	size, _ := fs.DiskUsage(true)
+	var size = atomic.LoadInt64(&fs.diskUsed)
+	// Sorry go gods. This is ugly but the best approach I can come up with for right
+	// now without completely re-evaluating the logic we use for determining disk space.
+	//
+	// Normally I would just be using the atomic load right below, but I'm not sure about
+	// the scenarios where it is 0 because nothing has run that would trigger a disk size
+	// calculation?
+	//
+	// Perhaps that isn't even a concern for the sake of this?
+	if !fs.isTest {
+		size, _ = fs.DiskUsage(true)
+	}
 
 	// If we're dropping below 0 somehow just cap it to 0.
 	if (size + i) < 0 {

server/filesystem/filesystem.go

@@ -2,8 +2,6 @@ package filesystem
 
 import (
 	"bufio"
-	"bytes"
-	"fmt"
 	"github.com/gabriel-vasile/mimetype"
 	"github.com/karrick/godirwalk"
 	"github.com/pkg/errors"
@@ -33,6 +31,8 @@ type Filesystem struct {
 
 	// The root data directory path for this Filesystem instance.
 	root string
+
+	isTest bool
 }
 
 // Creates a new Filesystem instance for a given server.
@@ -53,18 +53,27 @@ func (fs *Filesystem) Path() string {
 // Reads a file on the system and returns it as a byte representation in a file
 // reader. This is not the most memory efficient usage since it will be reading the
 // entirety of the file into memory.
-func (fs *Filesystem) Readfile(p string) (io.Reader, error) {
+func (fs *Filesystem) Readfile(p string, w io.Writer) error {
 	cleaned, err := fs.SafePath(p)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	b, err := ioutil.ReadFile(cleaned)
+	if st, err := os.Stat(cleaned); err != nil {
+		return err
+	} else if st.IsDir() {
+		return ErrIsDirectory
+	}
+
+	f, err := os.Open(cleaned)
 	if err != nil {
-		return nil, err
+		return err
 	}
+	defer f.Close()
 
-	return bytes.NewReader(b), nil
+	_, err = bufio.NewReader(f).WriteTo(w)
+
+	return err
 }
 
 // Writes a file to the system. If the file does not already exist one will be created.
@@ -161,7 +170,7 @@ func (fs *Filesystem) Rename(from string, to string) error {
 	// Ensure that the directory we're moving into exists correctly on the system. Only do this if
 	// we're not at the root directory level.
 	if d != fs.Path() {
-		if mkerr := os.MkdirAll(d, 0644); mkerr != nil {
+		if mkerr := os.MkdirAll(d, 0755); mkerr != nil {
 			return errors.Wrap(mkerr, "failed to create directory structure for file rename")
 		}
 	}
@@ -179,6 +188,10 @@ func (fs *Filesystem) Chown(path string) error {
 		return errors.WithStack(err)
 	}
 
+	if fs.isTest {
+		return nil
+	}
+
 	uid := config.Get().System.User.Uid
 	gid := config.Get().System.User.Gid
 
@@ -214,6 +227,42 @@ func (fs *Filesystem) Chown(path string) error {
 	})
 }
 
+// Begin looping up to 50 times to try and create a unique copy file name. This will take
+// an input of "file.txt" and generate "file copy.txt". If that name is already taken, it will
+// then try to write "file copy 2.txt" and so on, until reaching 50 loops. At that point we
+// won't waste anymore time, just use the current timestamp and make that copy.
+//
+// Could probably make this more efficient by checking if there are any files matching the copy
+// pattern, and trying to find the highest number and then incrementing it by one rather than
+// looping endlessly.
+func (fs *Filesystem) findCopySuffix(dir string, name string, extension string) (string, error) {
+	var i int
+	var suffix = " copy"
+
+	for i = 0; i < 51; i++ {
+		if i > 0 {
+			suffix = " copy " + strconv.Itoa(i)
+		}
+
+		n := name + suffix + extension
+		// If we stat the file and it does not exist that means we're good to create the copy. If it
+		// does exist, we'll just continue to the next loop and try again.
+		if _, err := fs.Stat(path.Join(dir, n)); err != nil {
+			if !os.IsNotExist(err) {
+				return "", err
+			}
+
+			break
+		}
+
+		if i == 50 {
+			suffix = "copy." + time.Now().Format(time.RFC3339)
+		}
+	}
+
+	return name + suffix + extension, nil
+}
+
 // Copies a given file to the same location and appends a suffix to the file to indicate that
 // it has been copied.
 func (fs *Filesystem) Copy(p string) error {
@@ -249,70 +298,21 @@ func (fs *Filesystem) Copy(p string) error {
 		name = strings.TrimSuffix(name, ".tar")
 	}
 
-	// Begin looping up to 50 times to try and create a unique copy file name. This will take
-	// an input of "file.txt" and generate "file copy.txt". If that name is already taken, it will
-	// then try to write "file copy 2.txt" and so on, until reaching 50 loops. At that point we
-	// won't waste anymore time, just use the current timestamp and make that copy.
-	//
-	// Could probably make this more efficient by checking if there are any files matching the copy
-	// pattern, and trying to find the highest number and then incrementing it by one rather than
-	// looping endlessly.
-	var i int
-	copySuffix := " copy"
-	for i = 0; i < 51; i++ {
-		if i > 0 {
-			copySuffix = " copy " + strconv.Itoa(i)
-		}
-
-		tryName := fmt.Sprintf("%s%s%s", name, copySuffix, extension)
-		tryLocation, err := fs.SafePath(path.Join(relative, tryName))
-		if err != nil {
-			return errors.WithStack(err)
-		}
-
-		// If the file exists, continue to the next loop, otherwise we're good to start a copy.
-		if _, err := os.Stat(tryLocation); err != nil && !os.IsNotExist(err) {
-			return errors.WithStack(err)
-		} else if os.IsNotExist(err) {
-			break
-		}
-
-		if i == 50 {
-			copySuffix = "." + time.Now().Format(time.RFC3339)
-		}
-	}
-
-	finalPath, err := fs.SafePath(path.Join(relative, fmt.Sprintf("%s%s%s", name, copySuffix, extension)))
-	if err != nil {
-		return errors.WithStack(err)
-	}
-
 	source, err := os.Open(cleaned)
 	if err != nil {
 		return errors.WithStack(err)
 	}
 	defer source.Close()
 
-	dest, err := os.Create(finalPath)
+	n, err := fs.findCopySuffix(relative, name, extension)
-	if err != nil {
-		return errors.WithStack(err)
-	}
-	defer dest.Close()
 
-	buf := make([]byte, 1024*4)
+	return fs.Writefile(path.Join(relative, n), source)
-	if _, err := io.CopyBuffer(dest, source, buf); err != nil {
-		return errors.WithStack(err)
-	}
-
-	// Once everything is done, increment the disk space used.
-	fs.addDisk(s.Size())
-
-	return nil
 }
 
 // Deletes a file or folder from the system. Prevents the user from accidentally
 // (or maliciously) removing their root server data directory.
 func (fs *Filesystem) Delete(p string) error {
+	wg := sync.WaitGroup{}
 	// This is one of the few (only?) places in the codebase where we're explicitly not using
 	// the SafePath functionality when working with user provided input. If we did, you would
 	// not be able to delete a file that is a symlink pointing to a location outside of the data
@@ -332,7 +332,7 @@ func (fs *Filesystem) Delete(p string) error {
 		return errors.New("cannot delete root server directory")
 	}
 
-	if st, err := os.Stat(resolved); err != nil {
+	if st, err := os.Lstat(resolved); err != nil {
 		if !os.IsNotExist(err) {
 			fs.error(err).Warn("error while attempting to stat file before deletion")
 		}
@@ -340,14 +340,18 @@ func (fs *Filesystem) Delete(p string) error {
 		if !st.IsDir() {
 			fs.addDisk(-st.Size())
 		} else {
-			go func(st os.FileInfo, resolved string) {
+			wg.Add(1)
+			go func(wg *sync.WaitGroup, st os.FileInfo, resolved string) {
+				defer wg.Done()
 				if s, err := fs.DirectorySize(resolved); err == nil {
 					fs.addDisk(-s)
 				}
-			}(st, resolved)
+			}(&wg, st, resolved)
 		}
 	}
 
+	wg.Wait()
+
 	return os.RemoveAll(resolved)
 }
 
@@ -408,7 +412,11 @@ func (fs *Filesystem) ListDirectory(p string) ([]*Stat, error) {
 			var m *mimetype.MIME
 			var d = "inode/directory"
 			if !f.IsDir() {
-				cleanedp, _ := fs.SafeJoin(cleaned, f)
+				cleanedp := filepath.Join(cleaned, f.Name())
+				if f.Mode()&os.ModeSymlink != 0 {
+					cleanedp, _ = fs.SafePath(filepath.Join(cleaned, f.Name()))
+				}
+
 				if cleanedp != "" {
 					m, _ = mimetype.DetectFile(filepath.Join(cleaned, f.Name()))
 				} else {

server/filesystem/filesystem_test.go

@@ -0,0 +1,784 @@
+package filesystem
+
+import (
+	"bytes"
+	"errors"
+	. "github.com/franela/goblin"
+	"github.com/pterodactyl/wings/config"
+	"io/ioutil"
+	"math/rand"
+	"os"
+	"path/filepath"
+	"testing"
+	"unicode/utf8"
+)
+
+func NewFs() (*Filesystem, *rootFs) {
+	config.Set(&config.Configuration{
+		AuthenticationToken: "abc",
+		System: config.SystemConfiguration{
+			RootDirectory:     "/server",
+			DiskCheckInterval: 150,
+		},
+	})
+
+	tmpDir, err := ioutil.TempDir(os.TempDir(), "pterodactyl")
+	if err != nil {
+		panic(err)
+	}
+	// defer os.RemoveAll(tmpDir)
+
+	rfs := rootFs{root: tmpDir}
+
+	rfs.reset()
+
+	fs := New(filepath.Join(tmpDir, "/server"), 0)
+	fs.isTest = true
+
+	return fs, &rfs
+}
+
+type rootFs struct {
+	root string
+}
+
+func (rfs *rootFs) CreateServerFile(p string, c string) error {
+	f, err := os.Create(filepath.Join(rfs.root, "/server", p))
+
+	if err == nil {
+		f.Write([]byte(c))
+		f.Close()
+	}
+
+	return err
+}
+
+func (rfs *rootFs) StatServerFile(p string) (os.FileInfo, error) {
+	return os.Stat(filepath.Join(rfs.root, "/server", p))
+}
+
+func (rfs *rootFs) reset() {
+	if err := os.RemoveAll(filepath.Join(rfs.root, "/server")); err != nil {
+		if !os.IsNotExist(err) {
+			panic(err)
+		}
+	}
+
+	if err := os.Mkdir(filepath.Join(rfs.root, "/server"), 0755); err != nil {
+		panic(err)
+	}
+}
+
+func TestFilesystem_Path(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+
+	g.Describe("Path", func() {
+		g.It("returns the root path for the instance", func() {
+			g.Assert(fs.Path()).Equal(filepath.Join(rfs.root, "/server"))
+		})
+	})
+}
+
+func TestFilesystem_SafePath(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+	prefix := filepath.Join(rfs.root, "/server")
+
+	g.Describe("SafePath", func() {
+		g.It("returns a cleaned path to a given file", func() {
+			p, err := fs.SafePath("test.txt")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix + "/test.txt")
+
+			p, err = fs.SafePath("/test.txt")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix + "/test.txt")
+
+			p, err = fs.SafePath("./test.txt")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix + "/test.txt")
+
+			p, err = fs.SafePath("/foo/../test.txt")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix + "/test.txt")
+
+			p, err = fs.SafePath("/foo/bar")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix + "/foo/bar")
+		})
+
+		g.It("handles root directory access", func() {
+			p, err := fs.SafePath("/")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix)
+
+			p, err = fs.SafePath("")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix)
+		})
+
+		g.It("removes trailing slashes from paths", func() {
+			p, err := fs.SafePath("/foo/bar/")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix + "/foo/bar")
+		})
+
+		g.It("handles deeply nested directories that do not exist", func() {
+			p, err := fs.SafePath("/foo/bar/baz/quaz/../../ducks/testing.txt")
+			g.Assert(err).IsNil()
+			g.Assert(p).Equal(prefix + "/foo/bar/ducks/testing.txt")
+		})
+
+		g.It("blocks access to files outside the root directory", func() {
+			p, err := fs.SafePath("../test.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+			g.Assert(p).Equal("")
+
+			p, err = fs.SafePath("/../test.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+			g.Assert(p).Equal("")
+
+			p, err = fs.SafePath("./foo/../../test.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+			g.Assert(p).Equal("")
+
+			p, err = fs.SafePath("..")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+			g.Assert(p).Equal("")
+		})
+	})
+}
+
+// We test against accessing files outside the root directory in the tests, however it
+// is still possible for someone to mess up and not properly use this safe path call. In
+// order to truly confirm this, we'll try to pass in a symlinked malicious file to all of
+// the calls and ensure they all fail with the same reason.
+func TestFilesystem_Blocks_Symlinks(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+
+	if err := rfs.CreateServerFile("/../malicious.txt", "external content"); err != nil {
+		panic(err)
+	}
+
+	if err := os.Mkdir(filepath.Join(rfs.root, "/malicious_dir"), 0777); err != nil {
+		panic(err)
+	}
+
+	if err := os.Symlink(filepath.Join(rfs.root, "malicious.txt"), filepath.Join(rfs.root, "/server/symlinked.txt")); err != nil {
+		panic(err)
+	}
+
+	if err := os.Symlink(filepath.Join(rfs.root, "/malicious_dir"), filepath.Join(rfs.root, "/server/external_dir")); err != nil {
+		panic(err)
+	}
+
+	g.Describe("Readfile", func() {
+		g.It("cannot read a file symlinked outside the root", func() {
+			b := bytes.Buffer{}
+
+			err := fs.Readfile("symlinked.txt", &b)
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+	})
+
+	g.Describe("Writefile", func() {
+		g.It("cannot write to a file symlinked outside the root", func() {
+			r := bytes.NewReader([]byte("testing"))
+
+			err := fs.Writefile("symlinked.txt", r)
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("cannot write a file to a directory symlinked outside the root", func() {
+			r := bytes.NewReader([]byte("testing"))
+
+			err := fs.Writefile("external_dir/foo.txt", r)
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+	})
+
+	g.Describe("CreateDirectory", func() {
+		g.It("cannot create a directory outside the root", func() {
+			err := fs.CreateDirectory("my_dir", "external_dir")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("cannot create a nested directory outside the root", func() {
+			err := fs.CreateDirectory("my/nested/dir", "external_dir/foo/bar")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("cannot create a nested directory outside the root", func() {
+			err := fs.CreateDirectory("my/nested/dir", "external_dir/server")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+	})
+
+	g.Describe("Rename", func() {
+		g.It("cannot rename a file symlinked outside the directory root", func() {
+			err := fs.Rename("symlinked.txt", "foo.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("cannot rename a symlinked directory outside the root", func() {
+			err := fs.Rename("external_dir", "foo")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("cannot rename a file to a location outside the directory root", func() {
+			rfs.CreateServerFile("my_file.txt", "internal content")
+
+			err := fs.Rename("my_file.txt", "external_dir/my_file.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+	})
+
+	g.Describe("Chown", func() {
+		g.It("cannot chown a file symlinked outside the directory root", func() {
+			err := fs.Chown("symlinked.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("cannot chown a directory symlinked outside the directory root", func() {
+			err := fs.Chown("external_dir")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+	})
+
+	g.Describe("Copy", func() {
+		g.It("cannot copy a file symlinked outside the directory root", func() {
+			err := fs.Copy("symlinked.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+	})
+
+	g.Describe("Delete", func() {
+		g.It("deletes the symlinked file but leaves the source", func() {
+			err := fs.Delete("symlinked.txt")
+			g.Assert(err).IsNil()
+
+			_, err = os.Stat(filepath.Join(rfs.root, "malicious.txt"))
+			g.Assert(err).IsNil()
+
+			_, err = rfs.StatServerFile("symlinked.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+		})
+	})
+
+	rfs.reset()
+}
+
+func TestFilesystem_Readfile(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+
+	g.Describe("Readfile", func() {
+		buf := &bytes.Buffer{}
+
+		g.It("opens a file if it exists on the system", func() {
+			err := rfs.CreateServerFile("test.txt", "testing")
+			g.Assert(err).IsNil()
+
+			err = fs.Readfile("test.txt", buf)
+			g.Assert(err).IsNil()
+			g.Assert(buf.String()).Equal("testing")
+		})
+
+		g.It("returns an error if the file does not exist", func() {
+			err := fs.Readfile("test.txt", buf)
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+		})
+
+		g.It("returns an error if the \"file\" is a directory", func() {
+			err := os.Mkdir(filepath.Join(rfs.root, "/server/test.txt"), 0755)
+			g.Assert(err).IsNil()
+
+			err = fs.Readfile("test.txt", buf)
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrIsDirectory)).IsTrue()
+		})
+
+		g.It("cannot open a file outside the root directory", func() {
+			err := rfs.CreateServerFile("/../test.txt", "testing")
+			g.Assert(err).IsNil()
+
+			err = fs.Readfile("/../test.txt", buf)
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.AfterEach(func() {
+			buf.Truncate(0)
+			fs.diskUsed = 0
+			rfs.reset()
+		})
+	})
+}
+
+func TestFilesystem_Writefile(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+
+	g.Describe("Open and WriteFile", func() {
+		buf := &bytes.Buffer{}
+
+		// Test that a file can be written to the disk and that the disk space used as a result
+		// is updated correctly in the end.
+		g.It("can create a new file", func() {
+			r := bytes.NewReader([]byte("test file content"))
+
+			g.Assert(fs.diskUsed).Equal(int64(0))
+
+			err := fs.Writefile("test.txt", r)
+			g.Assert(err).IsNil()
+
+			err = fs.Readfile("test.txt", buf)
+			g.Assert(err).IsNil()
+			g.Assert(buf.String()).Equal("test file content")
+			g.Assert(fs.diskUsed).Equal(r.Size())
+		})
+
+		g.It("can create a new file inside a nested directory with leading slash", func() {
+			r := bytes.NewReader([]byte("test file content"))
+
+			err := fs.Writefile("/some/nested/test.txt", r)
+			g.Assert(err).IsNil()
+
+			err = fs.Readfile("/some/nested/test.txt", buf)
+			g.Assert(err).IsNil()
+			g.Assert(buf.String()).Equal("test file content")
+		})
+
+		g.It("can create a new file inside a nested directory without a trailing slash", func() {
+			r := bytes.NewReader([]byte("test file content"))
+
+			err := fs.Writefile("some/../foo/bar/test.txt", r)
+			g.Assert(err).IsNil()
+
+			err = fs.Readfile("foo/bar/test.txt", buf)
+			g.Assert(err).IsNil()
+			g.Assert(buf.String()).Equal("test file content")
+		})
+
+		g.It("cannot create a file outside the root directory", func() {
+			r := bytes.NewReader([]byte("test file content"))
+
+			err := fs.Writefile("/some/../foo/../../test.txt", r)
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("cannot write a file that exceedes the disk limits", func() {
+			fs.diskLimit = 1024
+
+			b := make([]byte, 1025)
+			_, err := rand.Read(b)
+			g.Assert(err).IsNil()
+			g.Assert(len(b)).Equal(1025)
+
+			r := bytes.NewReader(b)
+			err = fs.Writefile("test.txt", r)
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrNotEnoughDiskSpace)).IsTrue()
+		})
+
+		g.It("updates the total space used when a file is appended to", func() {
+			fs.diskUsed = 100
+
+			b := make([]byte, 100)
+			_, _ = rand.Read(b)
+
+			r := bytes.NewReader(b)
+			err := fs.Writefile("test.txt", r)
+			g.Assert(err).IsNil()
+			g.Assert(fs.diskUsed).Equal(int64(200))
+
+			// If we write less data than already exists, we should expect the total
+			// disk used to be decremented.
+			b = make([]byte, 50)
+			_, _ = rand.Read(b)
+
+			r = bytes.NewReader(b)
+			err = fs.Writefile("test.txt", r)
+			g.Assert(err).IsNil()
+			g.Assert(fs.diskUsed).Equal(int64(150))
+		})
+
+		g.It("truncates the file when writing new contents", func() {
+			r := bytes.NewReader([]byte("original data"))
+			err := fs.Writefile("test.txt", r)
+			g.Assert(err).IsNil()
+
+			r = bytes.NewReader([]byte("new data"))
+			err = fs.Writefile("test.txt", r)
+			g.Assert(err).IsNil()
+
+			err = fs.Readfile("test.txt", buf)
+			g.Assert(err).IsNil()
+			g.Assert(buf.String()).Equal("new data")
+		})
+
+		g.AfterEach(func() {
+			buf.Truncate(0)
+			rfs.reset()
+			fs.diskUsed = 0
+			fs.diskLimit = 0
+		})
+	})
+}
+
+func TestFilesystem_CreateDirectory(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+
+	g.Describe("CreateDirectory", func() {
+		g.It("should create missing directories automatically", func() {
+			err := fs.CreateDirectory("test", "foo/bar/baz")
+			g.Assert(err).IsNil()
+
+			st, err := rfs.StatServerFile("foo/bar/baz/test")
+			g.Assert(err).IsNil()
+			g.Assert(st.IsDir()).IsTrue()
+			g.Assert(st.Name()).Equal("test")
+		})
+
+		g.It("should work with leading and trailing slashes", func() {
+			err := fs.CreateDirectory("test", "/foozie/barzie/bazzy/")
+			g.Assert(err).IsNil()
+
+			st, err := rfs.StatServerFile("foozie/barzie/bazzy/test")
+			g.Assert(err).IsNil()
+			g.Assert(st.IsDir()).IsTrue()
+			g.Assert(st.Name()).Equal("test")
+		})
+
+		g.It("should not allow the creation of directories outside the root", func() {
+			err := fs.CreateDirectory("test", "e/../../something")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("should not increment the disk usage", func() {
+			err := fs.CreateDirectory("test", "/")
+			g.Assert(err).IsNil()
+			g.Assert(fs.diskUsed).Equal(int64(0))
+		})
+
+		g.AfterEach(func() {
+			rfs.reset()
+		})
+	})
+}
+
+func TestFilesystem_Rename(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+
+	g.Describe("Rename", func() {
+		g.BeforeEach(func() {
+			if err := rfs.CreateServerFile("source.txt", "text content"); err != nil {
+				panic(err)
+			}
+		})
+
+		g.It("returns an error if the target already exists", func() {
+			err := rfs.CreateServerFile("target.txt", "taget content")
+			g.Assert(err).IsNil()
+
+			err = fs.Rename("source.txt", "target.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrExist)).IsTrue()
+		})
+
+		g.It("returns an error if the final destination is the root directory", func() {
+			err := fs.Rename("source.txt", "/")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrExist)).IsTrue()
+		})
+
+		g.It("returns an error if the source destination is the root directory", func() {
+			err := fs.Rename("source.txt", "/")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrExist)).IsTrue()
+		})
+
+		g.It("does not allow renaming to a location outside the root", func() {
+			err := fs.Rename("source.txt", "../target.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("does not allow renaming from a location outside the root", func() {
+			err := rfs.CreateServerFile("/../ext-source.txt", "taget content")
+
+			err = fs.Rename("/../ext-source.txt", "target.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("allows a file to be renamed", func() {
+			err := fs.Rename("source.txt", "target.txt")
+			g.Assert(err).IsNil()
+
+			_, err = rfs.StatServerFile("source.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+
+			st, err := rfs.StatServerFile("target.txt")
+			g.Assert(err).IsNil()
+			g.Assert(st.Name()).Equal("target.txt")
+			g.Assert(st.Size()).IsNotZero()
+		})
+
+		g.It("allows a folder to be renamed", func() {
+			err := os.Mkdir(filepath.Join(rfs.root, "/server/source_dir"), 0755)
+			g.Assert(err).IsNil()
+
+			err = fs.Rename("source_dir", "target_dir")
+			g.Assert(err).IsNil()
+
+			_, err = rfs.StatServerFile("source_dir")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+
+			st, err := rfs.StatServerFile("target_dir")
+			g.Assert(err).IsNil()
+			g.Assert(st.IsDir()).IsTrue()
+		})
+
+		g.It("returns an error if the source does not exist", func() {
+			err := fs.Rename("missing.txt", "target.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+		})
+
+		g.It("creates directories if they are missing", func() {
+			err := fs.Rename("source.txt", "nested/folder/target.txt")
+			g.Assert(err).IsNil()
+
+			st, err := rfs.StatServerFile("nested/folder/target.txt")
+			g.Assert(err).IsNil()
+			g.Assert(st.Name()).Equal("target.txt")
+		})
+
+		g.AfterEach(func() {
+			rfs.reset()
+		})
+	})
+}
+
+func TestFilesystem_Copy(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+
+	g.Describe("Copy", func() {
+		g.BeforeEach(func() {
+			if err := rfs.CreateServerFile("source.txt", "text content"); err != nil {
+				panic(err)
+			}
+
+			fs.diskUsed = int64(utf8.RuneCountInString("test content"))
+		})
+
+		g.It("should return an error if the source does not exist", func() {
+			err := fs.Copy("foo.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+		})
+
+		g.It("should return an error if the source is outside the root", func() {
+			err := rfs.CreateServerFile("/../ext-source.txt", "text content")
+
+			err = fs.Copy("../ext-source.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("should return an error if the source directory is outside the root", func() {
+			err := os.MkdirAll(filepath.Join(rfs.root, "/nested/in/dir"), 0755)
+			g.Assert(err).IsNil()
+
+			err = rfs.CreateServerFile("/../nested/in/dir/ext-source.txt", "external content")
+			g.Assert(err).IsNil()
+
+			err = fs.Copy("../nested/in/dir/ext-source.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+
+			err = fs.Copy("nested/in/../../../nested/in/dir/ext-source.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("should return an error if the source is a directory", func() {
+			err := os.Mkdir(filepath.Join(rfs.root, "/server/dir"), 0755)
+			g.Assert(err).IsNil()
+
+			err = fs.Copy("dir")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+		})
+
+		g.It("should return an error if there is not space to copy the file", func() {
+			fs.diskLimit = 2
+
+			err := fs.Copy("source.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrNotEnoughDiskSpace)).IsTrue()
+		})
+
+		g.It("should create a copy of the file and increment the disk used", func() {
+			err := fs.Copy("source.txt")
+			g.Assert(err).IsNil()
+
+			_, err = rfs.StatServerFile("source.txt")
+			g.Assert(err).IsNil()
+
+			_, err = rfs.StatServerFile("source copy.txt")
+			g.Assert(err).IsNil()
+		})
+
+		g.It("should create a copy of the file with a suffix if a copy already exists", func() {
+			err := fs.Copy("source.txt")
+			g.Assert(err).IsNil()
+
+			err = fs.Copy("source.txt")
+			g.Assert(err).IsNil()
+
+			r := []string{"source.txt", "source copy.txt", "source copy 1.txt"}
+
+			for _, name := range r {
+				_, err = rfs.StatServerFile(name)
+				g.Assert(err).IsNil()
+			}
+
+			g.Assert(fs.diskUsed).Equal(int64(utf8.RuneCountInString("test content")) * 3)
+		})
+
+		g.It("should create a copy inside of a directory", func() {
+			err := os.MkdirAll(filepath.Join(rfs.root, "/server/nested/in/dir"), 0755)
+			g.Assert(err).IsNil()
+
+			err = rfs.CreateServerFile("nested/in/dir/source.txt", "test content")
+			g.Assert(err).IsNil()
+
+			err = fs.Copy("nested/in/dir/source.txt")
+			g.Assert(err).IsNil()
+
+			_, err = rfs.StatServerFile("nested/in/dir/source.txt")
+			g.Assert(err).IsNil()
+
+			_, err = rfs.StatServerFile("nested/in/dir/source copy.txt")
+			g.Assert(err).IsNil()
+		})
+
+		g.AfterEach(func() {
+			rfs.reset()
+			fs.diskUsed = 0
+			fs.diskLimit = 0
+		})
+	})
+}
+
+func TestFilesystem_Delete(t *testing.T) {
+	g := Goblin(t)
+	fs, rfs := NewFs()
+
+	g.Describe("Delete", func() {
+		g.BeforeEach(func() {
+			if err := rfs.CreateServerFile("source.txt", "test content"); err != nil {
+				panic(err)
+			}
+
+			fs.diskUsed = int64(utf8.RuneCountInString("test content"))
+		})
+
+		g.It("does not delete files outside the root directory", func() {
+			err := rfs.CreateServerFile("/../ext-source.txt", "external content")
+
+			err = fs.Delete("../ext-source.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
+		})
+
+		g.It("does not allow the deletion of the root directory", func() {
+			err := fs.Delete("/")
+			g.Assert(err).IsNotNil()
+			g.Assert(err.Error()).Equal("cannot delete root server directory")
+		})
+
+		g.It("does not return an error if the target does not exist", func() {
+			err := fs.Delete("missing.txt")
+			g.Assert(err).IsNil()
+
+			st, err := rfs.StatServerFile("source.txt")
+			g.Assert(err).IsNil()
+			g.Assert(st.Name()).Equal("source.txt")
+		})
+
+		g.It("deletes files and subtracts their size from the disk usage", func() {
+			err := fs.Delete("source.txt")
+			g.Assert(err).IsNil()
+
+			_, err = rfs.StatServerFile("source.txt")
+			g.Assert(err).IsNotNil()
+			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+
+			g.Assert(fs.diskUsed).Equal(int64(0))
+		})
+
+		g.It("deletes all items inside a directory if the directory is deleted", func() {
+			sources := []string{
+				"foo/source.txt",
+				"foo/bar/source.txt",
+				"foo/bar/baz/source.txt",
+			}
+
+			err := os.MkdirAll(filepath.Join(rfs.root, "/server/foo/bar/baz"), 0755)
+			g.Assert(err).IsNil()
+
+			for _, s := range sources {
+				err = rfs.CreateServerFile(s, "test content")
+				g.Assert(err).IsNil()
+			}
+
+			fs.diskUsed = int64(utf8.RuneCountInString("test content") * 3)
+
+			err = fs.Delete("foo")
+			g.Assert(err).IsNil()
+			g.Assert(fs.diskUsed).Equal(int64(0))
+
+			for _, s := range sources {
+				_, err = rfs.StatServerFile(s)
+				g.Assert(err).IsNotNil()
+				g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
+			}
+		})
+
+		g.AfterEach(func() {
+			rfs.reset()
+			fs.diskUsed = 0
+			fs.diskLimit = 0
+		})
+	})
+}

server/filesystem/path.go

@@ -92,27 +92,6 @@ func (fs *Filesystem) unsafeIsInDataDirectory(p string) bool {
 	return strings.HasPrefix(strings.TrimSuffix(p, "/")+"/", strings.TrimSuffix(fs.Path(), "/")+"/")
 }
 
-// Helper function to keep some of the codebase a little cleaner. Returns a "safe" version of the path
-// joined with a file. This is important because you cannot just assume that appending a file to a cleaned
-// path will result in a cleaned path to that file. For example, imagine you have the following scenario:
-//
-// my_bad_file -> symlink:/etc/passwd
-//
-// cleaned := SafePath("../../etc") -> "/"
-// filepath.Join(cleaned, my_bad_file) -> "/my_bad_file"
-//
-// You might think that "/my_bad_file" is fine since it isn't pointing to the original "../../etc/my_bad_file".
-// However, this doesn't account for symlinks where the file might be pointing outside of the directory, so
-// calling a function such as Chown against it would chown the symlinked location, and not the file within the
-// Wings daemon.
-func (fs *Filesystem) SafeJoin(dir string, f os.FileInfo) (string, error) {
-	if f.Mode()&os.ModeSymlink != 0 {
-		return fs.SafePath(filepath.Join(dir, f.Name()))
-	}
-
-	return filepath.Join(dir, f.Name()), nil
-}
-
 // Executes the fs.SafePath function in parallel against an array of paths. If any of the calls
 // fails an error will be returned.
 func (fs *Filesystem) ParallelSafePath(paths []string) ([]string, error) {