diff --git a/router/middleware/middleware.go b/router/middleware/middleware.go index f659282..b63d49f 100644 --- a/router/middleware/middleware.go +++ b/router/middleware/middleware.go @@ -130,7 +130,7 @@ func (re *RequestError) asFilesystemError() (int, string) { return http.StatusBadRequest, "Cannot perform that action: file is a directory." } if filesystem.IsErrorCode(err, filesystem.ErrCodeDiskSpace) || strings.Contains(err.Error(), "filesystem: not enough disk space") { - return http.StatusBadRequest, "Cannot perform that action: file is a directory." + return http.StatusBadRequest, "There is not enough disk space available to perform that action." } if strings.HasSuffix(err.Error(), "file name too long") { return http.StatusBadRequest, "Cannot perform that action: file name is too long." diff --git a/router/router_server_files.go b/router/router_server_files.go index 7833e72..864eb1a 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -380,7 +380,8 @@ func postServerCompressFiles(c *gin.Context) { // of unpacking an archive that exists on the server into the provided RootPath // for the server. func postServerDecompressFiles(c *gin.Context) { - s := ExtractServer(c) + s := middleware.ExtractServer(c) + lg := middleware.ExtractLogger(c) var data struct { RootPath string `json:"root"` File string `json:"file"` @@ -389,12 +390,12 @@ func postServerDecompressFiles(c *gin.Context) { return } - lg := s.Log().WithFields(log.Fields{"root_path": data.RootPath, "file": data.File}) - lg.Debug("checking if space is available for decompression") - hasSpace, err := s.Filesystem().SpaceAvailableForDecompression(data.RootPath, data.File) + lg = lg.WithFields(log.Fields{"root_path": data.RootPath, "file": data.File}) + lg.Debug("checking if space is available for file decompression") + err := s.Filesystem().SpaceAvailableForDecompression(data.RootPath, data.File) if err != nil { if filesystem.IsErrorCode(err, filesystem.ErrCodeUnknownArchive) { - s.Log().WithField("path", data.RootPath).WithField("file", data.File).WithField("error", err).Warn("failed to decompress file due to unknown format") + lg.WithField("error", err).Warn("failed to decompress file: unknown archive format") c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": "The archive provided is in a format Wings does not understand."}) return } @@ -402,30 +403,21 @@ func postServerDecompressFiles(c *gin.Context) { return } - if !hasSpace { - c.AbortWithStatusJSON(http.StatusConflict, gin.H{ - "error": "This server does not have enough available disk space to decompress this archive.", - }) - return - } - + lg.Info("starting file decompression") if err := s.Filesystem().DecompressFile(data.RootPath, data.File); err != nil { // If the file is busy for some reason just return a nicer error to the user since there is not // much we specifically can do. They'll need to stop the running server process in order to overwrite // a file like this. if strings.Contains(err.Error(), "text file busy") { - s.Log().WithField("error", err).Warn("failed to decompress file due to busy text file") - + lg.WithField("error", err).Warn("failed to decompress file: text file busy") c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{ "error": "One or more files this archive is attempting to overwrite are currently in use by another process. Please try again.", }) return } - - NewServerError(err, s).Abort(c) + middleware.CaptureAndAbort(c, err) return } - c.Status(http.StatusNoContent) } diff --git a/server/filesystem/decompress.go b/server/filesystem/decompress.go index 4fb2988..51c3bff 100644 --- a/server/filesystem/decompress.go +++ b/server/filesystem/decompress.go @@ -4,28 +4,29 @@ import ( "archive/tar" "archive/zip" "compress/gzip" - "emperror.dev/errors" "fmt" - "github.com/mholt/archiver/v3" "os" "path/filepath" "reflect" "strings" "sync/atomic" + + "emperror.dev/errors" + "github.com/mholt/archiver/v3" ) -// Look through a given archive and determine if decompressing it would put the server over -// its allocated disk space limit. -func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (bool, error) { +// SpaceAvailableForDecompression looks through a given archive and determines +// if decompressing it would put the server over its allocated disk space limit. +func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) error { // Don't waste time trying to determine this if we know the server will have the space for // it since there is no limit. if fs.MaxDisk() <= 0 { - return true, nil + return nil } source, err := fs.SafePath(filepath.Join(dir, file)) if err != nil { - return false, err + return err } // Get the cached size in a parallel process so that if it is not cached we are not @@ -38,32 +39,28 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b if atomic.AddInt64(&size, f.Size())+dirSize > fs.MaxDisk() { return &Error{code: ErrCodeDiskSpace} } - return nil }) - if err != nil { if strings.HasPrefix(err.Error(), "format ") { - return false, &Error{code: ErrCodeUnknownArchive} + return &Error{code: ErrCodeUnknownArchive} } - - return false, err + return err } - - return true, err + return err } -// Decompress a file in a given directory by using the archiver tool to infer the file -// type and go from there. This will walk over all of the files within the given archive -// and ensure that there is not a zip-slip attack being attempted by validating that the -// final path is within the server data directory. +// DecompressFile will decompress a file in a given directory by using the +// archiver tool to infer the file type and go from there. This will walk over +// all of the files within the given archive and ensure that there is not a +// zip-slip attack being attempted by validating that the final path is within +// the server data directory. func (fs *Filesystem) DecompressFile(dir string, file string) error { source, err := fs.SafePath(filepath.Join(dir, file)) if err != nil { return err } - - // Make sure the file exists basically. + // Ensure that the source archive actually exists on the system. if _, err := os.Stat(source); err != nil { return err } @@ -79,7 +76,6 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error { } var name string - switch s := f.Sys().(type) { case *tar.Header: name = s.Name @@ -88,7 +84,11 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error { case *zip.FileHeader: name = s.Name default: - return errors.New(fmt.Sprintf("could not parse underlying data source with type %s", reflect.TypeOf(s).String())) + return &Error{ + code: ErrCodeUnknownError, + resolved: filepath.Join(dir, f.Name()), + err: errors.New(fmt.Sprintf("could not parse underlying data source with type: %s", reflect.TypeOf(s).String())), + } } p := filepath.Join(dir, name) @@ -96,15 +96,16 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error { if err := fs.IsIgnored(p); err != nil { return nil } - return errors.WithMessage(fs.Writefile(p, f), "could not extract file from archive") + if err := fs.Writefile(p, f); err != nil { + return &Error{code: ErrCodeUnknownError, err: err, resolved: source} + } + return nil }) if err != nil { if strings.HasPrefix(err.Error(), "format ") { return &Error{code: ErrCodeUnknownArchive} } - return err } - return nil } diff --git a/server/filesystem/errors.go b/server/filesystem/errors.go index 4cc9929..ac8159e 100644 --- a/server/filesystem/errors.go +++ b/server/filesystem/errors.go @@ -17,19 +17,33 @@ const ( ErrCodeUnknownArchive ErrorCode = "E_UNKNFMT" ErrCodePathResolution ErrorCode = "E_BADPATH" ErrCodeDenylistFile ErrorCode = "E_DENYLIST" + ErrCodeUnknownError ErrorCode = "E_UNKNOWN" ) type Error struct { - code ErrorCode - path string + code ErrorCode + // Contains the underlying error leading to this. This value may or may not be + // present, it is entirely dependent on how this error was triggered. + err error + // This contains the value of the final destination that triggered this specific + // error event. resolved string + // This value is generally only present on errors stemming from a path resolution + // error. For everything else you should be setting and reading the resolved path + // value which will be far more useful. + path string +} + +// Code returns the ErrorCode for this specific error instance. +func (e *Error) Code() ErrorCode { + return e.code } // Returns a human-readable error string to identify the Error by. func (e *Error) Error() string { switch e.code { case ErrCodeIsDirectory: - return "filesystem: is a directory" + return fmt.Sprintf("filesystem: cannot perform action: [%s] is a directory", e.resolved) case ErrCodeDiskSpace: return "filesystem: not enough disk space" case ErrCodeUnknownArchive: @@ -46,36 +60,17 @@ func (e *Error) Error() string { r = "" } return fmt.Sprintf("filesystem: server path [%s] resolves to a location outside the server root: %s", e.path, r) + case ErrCodeUnknownError: + fallthrough + default: + return fmt.Sprintf("filesystem: an error occurred: %s", e.Cause()) } - return "filesystem: unhandled error type" } -// Returns the ErrorCode for this specific error instance. -func (e *Error) Code() ErrorCode { - return e.code -} - -// Checks if the given error is one of the Filesystem errors. -func IsFilesystemError(err error) (*Error, bool) { - var fserr *Error - if errors.As(err, &fserr) { - return fserr, true - } - return nil, false -} - -// Checks if "err" is a filesystem Error type. If so, it will then drop in and check -// that the error code is the same as the provided ErrorCode passed in "code". -func IsErrorCode(err error, code ErrorCode) bool { - if e, ok := IsFilesystemError(err); ok { - return e.code == code - } - return false -} - -// Returns a new BadPathResolution error. -func NewBadPathResolution(path string, resolved string) *Error { - return &Error{code: ErrCodePathResolution, path: path, resolved: resolved} +// Cause returns the underlying cause of this filesystem error. In some causes +// there may not be a cause present, in which case nil will be returned. +func (e *Error) Cause() error { + return e.err } // Generates an error logger instance with some basic information. @@ -92,10 +87,46 @@ func (fs *Filesystem) handleWalkerError(err error, f os.FileInfo) error { if !IsErrorCode(err, ErrCodePathResolution) { return err } - if f != nil && f.IsDir() { return filepath.SkipDir } - return nil } + +// IsFilesystemError checks if the given error is one of the Filesystem errors. +func IsFilesystemError(err error) bool { + var fserr *Error + if err != nil && errors.As(err, &fserr) { + return true + } + return false +} + +// IsErrorCode checks if "err" is a filesystem Error type. If so, it will then +// drop in and check that the error code is the same as the provided ErrorCode +// passed in "code". +func IsErrorCode(err error, code ErrorCode) bool { + var fserr *Error + if err != nil && errors.As(err, &fserr) { + return fserr.code == code + } + return false +} + +// NewBadPathResolution returns a new BadPathResolution error. +func NewBadPathResolution(path string, resolved string) *Error { + return &Error{code: ErrCodePathResolution, path: path, resolved: resolved} +} + +// WrapError wraps the provided error as a Filesystem error and attaches the +// provided resolved source to it. If the error is already a Filesystem error +// no action is taken. +func WrapError(err error, resolved string) *Error { + if err == nil { + return nil + } + if IsFilesystemError(err) { + return err.(*Error) + } + return &Error{code: ErrCodeUnknownError, err: err, resolved: resolved} +} \ No newline at end of file diff --git a/server/filesystem/filesystem.go b/server/filesystem/filesystem.go index 4732a22..c5ebc1e 100644 --- a/server/filesystem/filesystem.go +++ b/server/filesystem/filesystem.go @@ -123,7 +123,8 @@ func (fs *Filesystem) Readfile(p string, w io.Writer) error { return err } -// Writes a file to the system. If the file does not already exist one will be created. +// Writefile writes a file to the system. If the file does not already exist one +// will be created. func (fs *Filesystem) Writefile(p string, r io.Reader) error { cleaned, err := fs.SafePath(p) if err != nil { @@ -138,7 +139,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error { return err } else if err == nil { if stat.IsDir() { - return &Error{code: ErrCodeIsDirectory} + return &Error{code: ErrCodeIsDirectory, resolved: cleaned} } currentSize = stat.Size() }