pkg/atomicwriter: validate destination path

- Disallow empty filenames
- Don't allow writing to a directory
- Return early if parent dir doesn't exist
- TBD: do we want to allow symlinks to be followed, or disallow?

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn
2025-03-10 15:05:33 +01:00
parent 084b7cec1a
commit 1daeaec333
2 changed files with 89 additions and 0 deletions

View File

@@ -2,16 +2,74 @@ package atomicwriter
import (
"errors"
"fmt"
"io"
"os"
"path/filepath"
)
func validateDestination(fileName string) error {
if fileName == "" {
return errors.New("file name is empty")
}
// Deliberately using Lstat here to match the behavior of [os.Rename],
// which is used when completing the write and does not resolve symlinks.
//
// TODO(thaJeztah): decide whether we want to disallow symlinks or to follow them.
if fi, err := os.Lstat(fileName); err != nil {
if !os.IsNotExist(err) {
return fmt.Errorf("failed to stat output path: %w", err)
}
} else if err := validateFileMode(fi.Mode()); err != nil {
return err
}
if dir := filepath.Dir(fileName); dir != "" && dir != "." {
if _, err := os.Stat(dir); errors.Is(err, os.ErrNotExist) {
return fmt.Errorf("invalid file path: %w", err)
}
}
return nil
}
func validateFileMode(mode os.FileMode) error {
switch {
case mode.IsRegular():
return nil // Regular file
case mode&os.ModeDir != 0:
return errors.New("cannot write to a directory")
// TODO(thaJeztah): decide whether we want to disallow symlinks or to follow them.
// case mode&os.ModeSymlink != 0:
// return errors.New("cannot write to a symbolic link directly")
case mode&os.ModeNamedPipe != 0:
return errors.New("cannot write to a named pipe (FIFO)")
case mode&os.ModeSocket != 0:
return errors.New("cannot write to a socket")
case mode&os.ModeDevice != 0:
if mode&os.ModeCharDevice != 0 {
return errors.New("cannot write to a character device file")
}
return errors.New("cannot write to a block device file")
case mode&os.ModeSetuid != 0:
return errors.New("cannot write to a setuid file")
case mode&os.ModeSetgid != 0:
return errors.New("cannot write to a setgid file")
case mode&os.ModeSticky != 0:
return errors.New("cannot write to a sticky bit file")
default:
// Unknown file mode; let's assume it works
return nil
}
}
// New returns a WriteCloser so that writing to it writes to a
// temporary file and closing it atomically changes the temporary file to
// destination path. Writing and closing concurrently is not allowed.
// NOTE: umask is not considered for the file's permissions.
func New(filename string, perm os.FileMode) (io.WriteCloser, error) {
if err := validateDestination(filename); err != nil {
return nil, err
}
abspath, err := filepath.Abs(filename)
if err != nil {
return nil, err

View File

@@ -120,9 +120,40 @@ func TestNewInvalid(t *testing.T) {
t.Errorf("Should produce a 'not found' error, but got %[1]T (%[1]v)", err)
}
})
t.Run("empty filename", func(t *testing.T) {
writer, err := New("", testMode())
if writer != nil {
t.Errorf("Should not have created writer")
}
if err == nil || err.Error() != "file name is empty" {
t.Errorf("Should produce a 'file name is empty' error, but got %[1]T (%[1]v)", err)
}
})
t.Run("directory", func(t *testing.T) {
tmpDir := t.TempDir()
writer, err := New(tmpDir, testMode())
if writer != nil {
t.Errorf("Should not have created writer")
}
if err == nil || err.Error() != "cannot write to a directory" {
t.Errorf("Should produce a 'cannot write to a directory' error, but got %[1]T (%[1]v)", err)
}
})
}
func TestWriteFile(t *testing.T) {
t.Run("empty filename", func(t *testing.T) {
err := WriteFile("", nil, testMode())
if err == nil || err.Error() != "file name is empty" {
t.Errorf("Should produce a 'file name is empty' error, but got %[1]T (%[1]v)", err)
}
})
t.Run("write to directory", func(t *testing.T) {
err := WriteFile(t.TempDir(), nil, testMode())
if err == nil || err.Error() != "cannot write to a directory" {
t.Errorf("Should produce a 'cannot write to a directory' error, but got %[1]T (%[1]v)", err)
}
})
t.Run("write to file", func(t *testing.T) {
tmpDir := t.TempDir()
fileName := filepath.Join(tmpDir, "test.txt")