fix(storage): Allow filesystem reads without write access

packages/shared/pkg/storage/storage_fs.go

@@ -126,7 +126,7 @@ func (o *fsObject) WriteTo(ctx context.Context, dst io.Writer) (n int64, err err
 	start := time.Now()
 	defer func() { RecordReadBlob(ctx, time.Since(start), n, o.path, SourceFS, err) }()
 
-	handle, err := o.getHandle(true)
+	handle, err := o.getHandle()
 	if err != nil {
 		return 0, err
 	}
@@ -225,7 +225,7 @@ func (o *fsObject) storeFileCompressed(ctx context.Context, localPath string, cf
 }
 
 func (o *fsObject) openRangeReader(_ context.Context, off, length int64) (RangeReader, error) {
-	f, err := o.getHandle(true)
+	f, err := o.getHandle()
 	if err != nil {
 		return nil, err
 	}
@@ -246,7 +246,7 @@ func (o *fsObject) Size(ctx context.Context) (_ int64, err error) {
 	start := time.Now()
 	defer func() { RecordReadSize(ctx, time.Since(start), o.objType, SourceFS, err) }()
 
-	handle, err := o.getHandle(true)
+	handle, err := o.getHandle()
 	if err != nil {
 		return 0, err
 	}
@@ -293,26 +293,27 @@ func ValidateUploadToken(key []byte, path string, expires int64, token string) b
 	return hmac.Equal([]byte(expected), []byte(token))
 }
 
-func (o *fsObject) getHandle(checkExistence bool) (*os.File, error) {
-	if checkExistence {
-		info, err := os.Stat(o.path)
-		if err != nil {
-			if os.IsNotExist(err) {
-				return nil, ErrObjectNotExist
-			}
-
-			return nil, err
+func (o *fsObject) getHandle() (*os.File, error) {
+	handle, err := os.Open(o.path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, ErrObjectNotExist
 		}
 
-		if info.IsDir() {
-			return nil, fmt.Errorf("path %s is a directory", o.path)
-		}
+		return nil, err
 	}
 
-	handle, err := os.OpenFile(o.path, os.O_RDWR|os.O_CREATE, 0o644)
+	info, err := handle.Stat()
 	if err != nil {
+		handle.Close()
+
 		return nil, err
 	}
+	if info.IsDir() {
+		handle.Close()
+
+		return nil, fmt.Errorf("path %s is a directory", o.path)
+	}
 
 	return handle, nil
 }

packages/shared/pkg/storage/storage_fs_test.go

@@ -1,8 +1,10 @@
 package storage
 
 import (
+	"bytes"
 	"context"
 	"errors"
+	"io"
 	"os"
 	"path/filepath"
 	"testing"
@@ -434,3 +436,74 @@ func TestWriteToNonExistentObject(t *testing.T) {
 	_, err = GetBlob(t.Context(), obj)
 	require.ErrorIs(t, err, ErrObjectNotExist)
 }
+
+func TestFSReadOnlyObject(t *testing.T) {
+	t.Parallel()
+	if os.Geteuid() == 0 {
+		t.Skip("root bypasses file permissions")
+	}
+
+	p := newTempProvider(t)
+	path := filepath.Join(p.basePath, "object")
+	const contents = "hello world"
+	require.NoError(t, os.WriteFile(path, []byte(contents), 0o444))
+	require.NoError(t, os.Chmod(p.basePath, 0o555))
+	t.Cleanup(func() {
+		require.NoError(t, os.Chmod(p.basePath, 0o755))
+	})
+
+	t.Run("WriteTo", func(t *testing.T) {
+		obj, err := p.OpenBlob(t.Context(), "object")
+		require.NoError(t, err)
+		var dst bytes.Buffer
+		n, err := obj.WriteTo(t.Context(), &dst)
+		require.NoError(t, err)
+		assert.EqualValues(t, len(contents), n)
+		assert.Equal(t, contents, dst.String())
+	})
+	t.Run("Size", func(t *testing.T) {
+		obj, err := p.OpenSeekable(t.Context(), "object")
+		require.NoError(t, err)
+		size, err := obj.Size(t.Context())
+		require.NoError(t, err)
+		assert.EqualValues(t, len(contents), size)
+	})
+	t.Run("OpenRangeReader", func(t *testing.T) {
+		obj, err := p.OpenSeekable(t.Context(), "object")
+		require.NoError(t, err)
+		reader, _, err := obj.OpenRangeReader(t.Context(), 6, 5, nil)
+		require.NoError(t, err)
+		defer reader.Close(t.Context())
+		data, err := io.ReadAll(reader)
+		require.NoError(t, err)
+		assert.Equal(t, "world", string(data))
+	})
+}
+
+func TestFSReadMissingObject(t *testing.T) {
+	t.Parallel()
+	p := newTempProvider(t)
+	obj, err := p.OpenSeekable(t.Context(), "missing")
+	require.NoError(t, err)
+
+	_, err = obj.Size(t.Context())
+	require.ErrorIs(t, err, ErrObjectNotExist)
+	reader, _, err := obj.OpenRangeReader(t.Context(), 0, 1, nil)
+	require.ErrorIs(t, err, ErrObjectNotExist)
+	assert.Nil(t, reader)
+	assert.NoFileExists(t, filepath.Join(p.basePath, "missing"))
+}
+
+func TestFSReadDirectory(t *testing.T) {
+	t.Parallel()
+	p := newTempProvider(t)
+	require.NoError(t, os.Mkdir(filepath.Join(p.basePath, "directory"), 0o755))
+	obj, err := p.OpenSeekable(t.Context(), "directory")
+	require.NoError(t, err)
+
+	_, err = obj.Size(t.Context())
+	require.ErrorContains(t, err, "is a directory")
+	reader, _, err := obj.OpenRangeReader(t.Context(), 0, 1, nil)
+	require.ErrorContains(t, err, "is a directory")
+	assert.Nil(t, reader)
+}