Skip to content

🔥 feat: Add support for RootDir and RootFs #4035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gaby wants to merge 10 commits into
base: main
Choose a base branch
from
Open

🔥 feat: Add support for RootDir and RootFs #4035

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e5d4bf3
Improve upload path validation errors
gaby Jan 26, 2026
06e9506
Add RootPerms for upload roots
gaby Jan 26, 2026
27bd252
Refine upload root fs handling
gaby Jan 26, 2026
2ddecf0
Fix upload path dot preservation
gaby Jan 27, 2026
6722830
Increase upload validation coverage
gaby Jan 27, 2026
030c269
Move upload tests into app_test
gaby Jan 27, 2026
ca7e850
test: align log capture and platform skips
gaby Jan 27, 2026
9e92226
test: cover SaveFile storage rootfs paths
gaby Jan 27, 2026
09cb0c8
docs: clarify SaveFileToStorage root prefix
gaby Jan 27, 2026
0d76119
Merge branch 'main' into add-path-normalization-and-validation-in-sav…
gaby Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"net"
"net/http"
"net/http/httputil"
Expand Down Expand Up @@ -169,6 +170,18 @@ type Config struct { //nolint:govet // Aligning the struct fields is not necessa
// Default: nil
Views Views `json:"-"`

// RootDir specifies the base directory for SaveFile/SaveFileToStorage uploads.
// Relative paths are resolved against this directory.
//
// Optional. Default: ""
RootDir string `json:"root_dir"`

// RootFs specifies the filesystem used for SaveFile/SaveFileToStorage uploads.
// When set, RootDir is treated as a relative prefix within the filesystem.
//
// Optional. Default: nil
RootFs fs.FS `json:"-"`

// Views Layout is the global layout for all template render until override on Render function.
//
// Default: ""
Expand Down Expand Up @@ -423,6 +436,12 @@ type Config struct { //nolint:govet // Aligning the struct fields is not necessa
//
// Optional. Default: a provider that returns context.Background()
ServicesShutdownContextProvider func() context.Context

uploadRootDir string
uploadRootEval string
uploadRootPath string
uploadRootFSPrefix string
uploadRootFSWriter uploadFSWriter
}

// Default TrustProxyConfig
Expand Down Expand Up @@ -615,6 +634,8 @@ func New(config ...Config) *App {
app.config.RequestMethods = DefaultMethods
}

app.configureUploads()

app.config.TrustProxyConfig.ips = make(map[string]struct{}, len(app.config.TrustProxyConfig.Proxies))
for _, ipAddress := range app.config.TrustProxyConfig.Proxies {
app.handleTrustedProxy(ipAddress)
Expand Down
90 changes: 87 additions & 3 deletions ctx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ package fiber
import (
"context"
"crypto/tls"
"errors"
"fmt"
"io"
"maps"
"mime/multipart"
"os"
"path/filepath"
"strconv"
"strings"
"sync/atomic"
Expand Down Expand Up @@ -496,12 +499,57 @@ func (c *DefaultCtx) IsPreflight() bool {
}

// SaveFile saves any multipart file to disk.
func (*DefaultCtx) SaveFile(fileheader *multipart.FileHeader, path string) error {
return fasthttp.SaveMultipartFile(fileheader, path)
func (c *DefaultCtx) SaveFile(fileheader *multipart.FileHeader, path string) error {
normalized, err := validateUploadPath(path)
if err != nil {
return err
}

if c.app.config.RootFs != nil {
fsPath := storageUploadPath(c.app.config.uploadRootFSPrefix, normalized.slashPath)
err = ensureNoSymlinkFS(c.app.config.RootFs, fsPath)
if err != nil {
return err
Comment thread
gaby marked this conversation as resolved.
}
return saveMultipartFileToFS(fileheader, fsPath, c.app.config.uploadRootFSWriter)
}

fullPath := normalized.osPath
if root := c.app.config.uploadRootDir; root != "" {
fullPath = filepath.Join(root, normalized.osPath)
err = ensureUploadPathWithinRoot(c.app.config.uploadRootEval, fullPath)
if err != nil {
return err
}
}

return fasthttp.SaveMultipartFile(fileheader, fullPath)
}

// SaveFileToStorage saves any multipart file to an external storage system.
func (c *DefaultCtx) SaveFileToStorage(fileheader *multipart.FileHeader, path string, storage Storage) error {
normalized, err := validateUploadPath(path)
if err != nil {
return err
}

if c.app.config.RootFs != nil {
fsPath := storageUploadPath(c.app.config.uploadRootFSPrefix, normalized.slashPath)
err = ensureNoSymlinkFS(c.app.config.RootFs, fsPath)
if err != nil {
return err
}
}
if root := c.app.config.uploadRootDir; root != "" {
fullPath := filepath.Join(root, filepath.FromSlash(normalized.slashPath))
err = ensureUploadPathWithinRoot(c.app.config.uploadRootEval, fullPath)
if err != nil {
return err
}
}

storagePath := storageUploadPath(c.app.config.uploadRootPath, normalized.slashPath)

file, err := fileheader.Open()
if err != nil {
return fmt.Errorf("failed to open: %w", err)
Expand Down Expand Up @@ -531,13 +579,49 @@ func (c *DefaultCtx) SaveFileToStorage(fileheader *multipart.FileHeader, path st

data := append([]byte(nil), buf.Bytes()...)

if err := storage.SetWithContext(c.Context(), path, data, 0); err != nil {
if err := storage.SetWithContext(c.Context(), storagePath, data, 0); err != nil {
return fmt.Errorf("failed to store: %w", err)
}

return nil
}

func saveMultipartFileToFS(
fileheader *multipart.FileHeader,
path string,
fsys uploadFSWriter,
) error {
file, err := fileheader.Open()
if err != nil {
return fmt.Errorf("failed to open multipart file: %w", err)
}
defer file.Close() //nolint:errcheck // not needed

dst, err := fsys.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)
if err != nil {
return fmt.Errorf("failed to open upload destination: %w", err)
}
writer, ok := dst.(io.Writer)
if !ok {
closeErr := dst.Close()
if closeErr != nil {
return fmt.Errorf("failed to close upload destination: %w", closeErr)
}
return errors.New("failed to open upload destination for write")
}
if _, err = io.Copy(writer, file); err != nil {
closeErr := dst.Close()
if closeErr != nil {
return fmt.Errorf("failed to close upload destination: %w", closeErr)
}
return fmt.Errorf("failed to copy upload data: %w", err)
}
Comment on lines +631 to +637
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Original copy error is lost when close also fails.

When io.Copy fails and dst.Close() also fails, only the close error is returned. The original copy error provides more diagnostic value. Consider using errors.Join or wrapping both.

Proposed fix to preserve both errors 
 	if _, err = io.Copy(writer, file); err != nil {
 		closeErr := dst.Close()
 		if closeErr != nil {
-			return fmt.Errorf("failed to close upload destination: %w", closeErr)
+			return fmt.Errorf("failed to copy upload data: %w (also failed to close: %v)", err, closeErr)
 		}
 		return fmt.Errorf("failed to copy upload data: %w", err)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if _, err = io.Copy(writer, file); err != nil {
closeErr := dst.Close()
if closeErr != nil {
return fmt.Errorf("failed to close upload destination: %w", closeErr)
}
return fmt.Errorf("failed to copy upload data: %w", err)
}
if _, err = io.Copy(writer, file); err != nil {
closeErr := dst.Close()
if closeErr != nil {
return fmt.Errorf("failed to copy upload data: %w (also failed to close: %v)", err, closeErr)
}
return fmt.Errorf("failed to copy upload data: %w", err)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@ctx.go` around lines 631 - 637, The current error handling in the io.Copy
failure path discards the original copy error when dst.Close() also fails;
update the block that handles io.Copy(writer, file) in ctx.go to preserve both
errors by capturing the copy error (err) and the close error (closeErr) and
returning a combined error (e.g., using errors.Join(copyErr, closeErr) or
wrapping them together) instead of returning only closeErr; ensure references to
writer, file, dst.Close(), and io.Copy remain and that the returned error
message includes both failure contexts.

if err := dst.Close(); err != nil {
return fmt.Errorf("failed to close upload destination: %w", err)
}
return nil
}

// Secure returns whether a secure connection was established.
func (c *DefaultCtx) Secure() bool {
return c.Protocol() == schemeHTTPS
Expand Down
135 changes: 123 additions & 12 deletions ctx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4230,25 +4230,20 @@ func Test_Ctx_RouteNormalized(t *testing.T) {
func Test_Ctx_SaveFile(t *testing.T) {
// TODO We should clean this up
t.Parallel()
app := New()
rootDir := t.TempDir()
app := New(Config{RootDir: rootDir})

app.Post("/test", func(c Ctx) error {
fh, err := c.Req().FormFile("file")
require.NoError(t, err)

tempFile, err := os.CreateTemp(os.TempDir(), "test-")
require.NoError(t, err)

defer func(file *os.File) {
closeErr := file.Close()
require.NoError(t, closeErr)
closeErr = os.Remove(file.Name())
require.NoError(t, closeErr)
}(tempFile)
err = c.SaveFile(fh, tempFile.Name())
relativePath := "upload.txt"
err = c.SaveFile(fh, relativePath)
require.NoError(t, err)

bs, err := os.ReadFile(tempFile.Name())
targetPath := filepath.Join(rootDir, relativePath)
// #nosec G304 -- reading from test-controlled temp directory.
bs, err := os.ReadFile(targetPath)
require.NoError(t, err)
require.Equal(t, "hello world", string(bs))
return nil
Expand All @@ -4273,6 +4268,57 @@ func Test_Ctx_SaveFile(t *testing.T) {
require.Equal(t, StatusOK, resp.StatusCode, "Status code")
}

// go test -run Test_Ctx_SaveFile_RootDirTraversal
func Test_Ctx_SaveFile_RootDirTraversal(t *testing.T) {
t.Parallel()

tests := map[string]string{
"traversal": filepath.Join("..", "outside.txt"),
"absolute": filepath.Join(t.TempDir(), "abs.txt"),
}

for name, targetPath := range tests {
Comment thread
gaby marked this conversation as resolved.
t.Run(name, func(t *testing.T) {
t.Parallel()
rootDir := t.TempDir()
app := New(Config{RootDir: rootDir})

app.Post("/test", func(c Ctx) error {
fh, err := c.FormFile("file")
require.NoError(t, err)

err = c.SaveFile(fh, targetPath)
require.Error(t, err)
return c.SendStatus(StatusOK)
})
Comment on lines +4801 to +4808
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Move these traversal assertions out of the handler and lock the exported error down.

These new RootDir traversal tests only check require.Error(...), so they will still pass on the wrong failure mode, and they add require calls inside Fiber handlers. Converting them to direct ctx.SaveFile* tests would keep assertions in the test goroutine and let each case assert require.ErrorIs(...) against the intended exported upload error.

Based on learnings: In the Fiber codebase, the linter does not allow require assertions from within HTTP handlers (including net/http-style handlers). Use t.Fatalf, t.Errorf, or similar testing.T methods for error handling inside handler functions instead.

Also applies to: 5024-5031

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@ctx_test.go` around lines 4801 - 4808, The tests currently perform traversal
assertions inside an HTTP handler by calling c.FormFile and c.SaveFile and using
require.NoError/require.Error there; move those assertions out of the handler so
they run in the test goroutine: call the handler-equivalent API directly (e.g.,
invoke ctx.SaveFile or the exported SaveFile helper) from the test and assert
with require.ErrorIs against the exported upload/traversal error constant
instead of a generic require.Error; if you must keep an in-handler check,
replace require.* calls inside the handler with testing.T methods
(t.Fatalf/t.Errorf) to avoid linter issues, but preferred fix is converting the
test to directly call SaveFile/FormFile equivalents and assert ErrorIs against
the exported upload error symbol.


body := &bytes.Buffer{}
writer := multipart.NewWriter(body)
ioWriter, err := writer.CreateFormFile("file", "test")
require.NoError(t, err)
_, err = ioWriter.Write([]byte("hello world"))
require.NoError(t, err)
require.NoError(t, writer.Close())

req := httptest.NewRequest(MethodPost, "/test", body)
req.Header.Set("Content-Type", writer.FormDataContentType())
req.Header.Set("Content-Length", strconv.Itoa(len(body.Bytes())))

resp, err := app.Test(req)
require.NoError(t, err, "app.Test(req)")
require.Equal(t, StatusOK, resp.StatusCode, "Status code")

expectedPath := targetPath
if !filepath.IsAbs(expectedPath) {
expectedPath = filepath.Join(rootDir, expectedPath)
}
expectedPath = filepath.Clean(expectedPath)
_, statErr := os.Stat(expectedPath)
require.Error(t, statErr)
})
}
}

func createMultipartFileHeader(t *testing.T, filename string, data []byte) *multipart.FileHeader {
t.Helper()

Expand Down Expand Up @@ -4300,6 +4346,25 @@ func createMultipartFileHeader(t *testing.T, filename string, data []byte) *mult
return files[0]
}

type recordingStorage struct {
*memory.Storage
setKeys []string
}

func newRecordingStorage() *recordingStorage {
return &recordingStorage{Storage: memory.New()}
}

func (s *recordingStorage) SetWithContext(ctx context.Context, key string, val []byte, exp time.Duration) error {
s.setKeys = append(s.setKeys, key)
return s.Storage.SetWithContext(ctx, key, val, exp)
}

func (s *recordingStorage) Set(key string, val []byte, exp time.Duration) error {
s.setKeys = append(s.setKeys, key)
return s.Storage.Set(key, val, exp)
}

// go test -run Test_Ctx_SaveFileToStorage
func Test_Ctx_SaveFileToStorage(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -4342,6 +4407,52 @@ func Test_Ctx_SaveFileToStorage(t *testing.T) {
require.Equal(t, StatusOK, resp.StatusCode, "Status code")
}

// go test -run Test_Ctx_SaveFileToStorage_RootDirTraversal
func Test_Ctx_SaveFileToStorage_RootDirTraversal(t *testing.T) {
t.Parallel()

tests := map[string]string{
"traversal": filepath.Join("..", "outside"),
"absolute": filepath.Join(t.TempDir(), "abs"),
}

for name, targetPath := range tests {
Comment thread
gaby marked this conversation as resolved.
t.Run(name, func(t *testing.T) {
t.Parallel()
rootDir := t.TempDir()
storage := newRecordingStorage()
app := New(Config{RootDir: rootDir})

app.Post("/test", func(c Ctx) error {
fh, err := c.FormFile("file")
require.NoError(t, err)

err = c.SaveFileToStorage(fh, targetPath, storage)
require.Error(t, err)
return c.SendStatus(StatusOK)
})

body := &bytes.Buffer{}
writer := multipart.NewWriter(body)
ioWriter, err := writer.CreateFormFile("file", "test")
require.NoError(t, err)
_, err = ioWriter.Write([]byte("hello world"))
require.NoError(t, err)
require.NoError(t, writer.Close())

req := httptest.NewRequest(MethodPost, "/test", body)
req.Header.Set("Content-Type", writer.FormDataContentType())
req.Header.Set("Content-Length", strconv.Itoa(len(body.Bytes())))

resp, err := app.Test(req)
require.NoError(t, err, "app.Test(req)")
require.Equal(t, StatusOK, resp.StatusCode, "Status code")

require.Empty(t, storage.setKeys)
})
}
}

func Test_Ctx_SaveFileToStorage_LargeUpload(t *testing.T) {
t.Parallel()
const (
Expand Down
4 changes: 4 additions & 0 deletions docs/api/ctx.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1710,6 +1710,8 @@ Method is used to save **any** multipart file to disk.
func (c fiber.Ctx) SaveFile(fh *multipart.FileHeader, path string) error
```

Paths must be relative and cannot contain `..` segments or absolute prefixes. When `Config.RootDir` or `Config.RootFs` is set, the path is resolved against that root and attempts to escape it are rejected.

Comment thread
gaby marked this conversation as resolved.
```go title="Example"
app.Post("/", func(c fiber.Ctx) error {
// Parse the multipart form:
Expand Down Expand Up @@ -1743,6 +1745,8 @@ Method is used to save **any** multipart file to an external storage system.
func (c fiber.Ctx) SaveFileToStorage(fileheader *multipart.FileHeader, path string, storage Storage) error
```

Paths must be relative and cannot contain `..` segments or absolute prefixes. When `Config.RootDir` or `Config.RootFs` is set, the path is resolved against that root and attempts to escape it are rejected.

```go title="Example"
storage := memory.New()

Expand Down
2 changes: 2 additions & 0 deletions docs/api/fiber.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ app := fiber.New(fiber.Config{
| <Reference id="readtimeout">ReadTimeout</Reference> | `time.Duration` | The amount of time allowed to read the full request, including the body. The default timeout is unlimited. | `nil` |
| <Reference id="reducememoryusage">ReduceMemoryUsage</Reference> | `bool` | Aggressively reduces memory usage at the cost of higher CPU usage if set to true. | `false` |
| <Reference id="requestmethods">RequestMethods</Reference> | `[]string` | RequestMethods provides customizability for HTTP methods. You can add/remove methods as you wish. | `DefaultMethods` |
| <Reference id="rootdir">RootDir</Reference> | `string` | Base directory for SaveFile/SaveFileToStorage uploads. Relative paths are resolved against this directory and must not escape it. | `""` |
| <Reference id="rootfs">RootFs</Reference> | `fs.FS` | Filesystem used for SaveFile/SaveFileToStorage uploads. When set, RootDir is treated as a relative prefix within the filesystem. | `nil` |
Comment thread
gaby marked this conversation as resolved.
| <Reference id="serverheader">ServerHeader</Reference> | `string` | Enables the `Server` HTTP header with the given value. | `""` |
| <Reference id="streamrequestbody">StreamRequestBody</Reference> | `bool` | StreamRequestBody enables request body streaming, and calls the handler sooner when given body is larger than the current limit. | `false` |
| <Reference id="strictrouting">StrictRouting</Reference> | `bool` | When enabled, the router treats `/foo` and `/foo/` as different. Otherwise, the router treats `/foo` and `/foo/` as the same. | `false` |
Expand Down
4 changes: 4 additions & 0 deletions error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ var (
ErrNoViewEngineConfigured = errors.New("fiber: no view engine configured")
// ErrAutoCertWithCertFile indicates AutoCertManager cannot be used with CertFile/CertKeyFile.
ErrAutoCertWithCertFile = errors.New("tls: AutoCertManager cannot be combined with CertFile/CertKeyFile")
// ErrInvalidUploadPath indicates the upload path is invalid.
ErrInvalidUploadPath = errors.New("upload: invalid path")
// ErrUploadPathEscapesRoot indicates the upload path escapes the configured root.
ErrUploadPathEscapesRoot = errors.New("upload: path escapes root")
)

// Fiber redirection errors
Expand Down
Loading
Loading