Merge 387383f19d into 6fd8457e5a

backend/app/api/handlers/v1/v1_ctrl_items_attachments.go

@@ -1,7 +1,9 @@
 package v1
 
 import (
+	"bytes"
 	"errors"
+	"io"
 	"net/http"
 	"path/filepath"
 	"strings"
@@ -157,13 +159,23 @@ func (ctrl *V1Controller) handleItemAttachmentsHandler(w http.ResponseWriter, r
 	ctx := services.NewContext(r.Context())
 	switch r.Method {
 	case http.MethodGet:
-		doc, err := ctrl.svc.Items.AttachmentPath(r.Context(), attachmentID)
+		attachment, content, err := ctrl.svc.Items.AttachmentData(r.Context(), attachmentID)
 		if err != nil {
-			log.Err(err).Msg("failed to get attachment path")
+			log.Err(err).Msg("failed to get attachment data")
 			return validate.NewRequestError(err, http.StatusInternalServerError)
 		}
 
-		http.ServeFile(w, r, doc.Path)
+		defer func() {
+			_ = content.Close()
+		}()
+
+		buf, err := io.ReadAll(content)
+		if err != nil {
+			log.Err(err).Msg("failed to buffer attachment contents")
+			return validate.NewRequestError(err, http.StatusInternalServerError)
+		}
+
+		http.ServeContent(w, r, attachment.Title, attachment.UpdatedAt, bytes.NewReader(buf))
 		return nil
 
 	// Delete Attachment Handler

backend/app/api/main.go

@@ -75,11 +75,6 @@ func run(cfg *config.Config) error {
 	// =========================================================================
 	// Initialize Database & Repos
 
-	err := os.MkdirAll(cfg.Storage.Data, 0o755)
-	if err != nil {
-		log.Fatal().Err(err).Msg("failed to create data directory")
-	}
-
 	c, err := ent.Open("sqlite3", cfg.Storage.SqliteURL)
 	if err != nil {
 		log.Fatal().

backend/internal/core/blobstore/local.go

@@ -0,0 +1,84 @@
+package blobstore
+
+import (
+	"context"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/hay-kot/homebox/backend/pkgs/pathlib"
+	"github.com/rs/zerolog/log"
+)
+
+// localBlobStore is a blob store implementation backed by the local filesystem.
+// Blob R/W operations translate to local file create, read, and write operations.
+type localBlobStore struct {
+	root string
+}
+
+// NewLocalBlobStore creates a local blob store rooted at the specified root directory.
+// Keys created, written, and deleted are relative to this root directory.
+func NewLocalBlobStore(root string) BlobStore {
+	err := os.MkdirAll(root, 0o755)
+	if err != nil {
+		log.Fatal().Err(err).Msg("failed to create data directory")
+	}
+
+	return &localBlobStore{
+		root: root,
+	}
+}
+
+func (l *localBlobStore) Get(ctx context.Context, key string) (io.ReadCloser, error) {
+	return os.Open(l.resolvePath(key))
+}
+
+func (l *localBlobStore) Put(ctx context.Context, key string, content io.Reader) (string, error) {
+	path := pathlib.Safe(l.resolvePath(key))
+
+	parent := filepath.Dir(path)
+	err := os.MkdirAll(parent, 0o755)
+	if err != nil {
+		return "", err
+	}
+
+	f, err := os.Create(path)
+	if err != nil {
+		return "", err
+	}
+
+	_, err = io.Copy(f, content)
+	if err != nil {
+		return "", err
+	}
+
+	return key, nil
+}
+
+func (l *localBlobStore) Delete(ctx context.Context, key string) error {
+	return os.Remove(l.resolvePath(key))
+}
+
+// resolvePath resolves the full path that corresponds to a blob key, taking the root directory
+// into account.
+func (l *localBlobStore) resolvePath(key string) string {
+	// XXX: A previous iteration of the document storage implementation persisted document paths
+	// with its fully qualified filesystem path, which included its root directory as a prefix.
+	// This compromised relocation resiliency of the attachment storage directory.
+	//
+	// For example, a root directory of "/usr/share/homebox" and blob key "gid/documents/id"
+	// would be persisted with identifier "/usr/share/homebox/gid/documents/id". This would
+	// break file integrity if "/usr/share/homebox" were relocated to "/usr/share/homebox2",
+	// even if the runtime storage root directory were changed to "/usr/share/homebox2".
+	//
+	// The current local storage implementation persists blob keys only, independent of the
+	// root path, which fixes this capability. However, to preserve backwards compatibility with
+	// existing documents written with prior behavior, assume that any blob keys that are
+	// prefixed with the root path are already fully resolved/qualified into filesystem paths.
+	if strings.HasPrefix(key, l.root) {
+		return key
+	}
+
+	return filepath.Join(l.root, key)
+}

backend/internal/core/blobstore/store.go

@@ -0,0 +1,25 @@
+// Package blobstore provides blob storage abstractions for reading and writing binary blobs.
+package blobstore
+
+import (
+	"context"
+	"io"
+)
+
+// BlobStore is an interface that describes a key-value-oriented blob storage backend for arbitrary
+// binary objects.
+//
+// Keys in the blob store are implementation-defined unique identifiers for locating a blob.
+type BlobStore interface {
+	// Get retrieves a blob by key, returning an io.ReadCloser capable of streaming the blob
+	// contents. Callers should close the returned blob to avoid leaks.
+	Get(ctx context.Context, key string) (io.ReadCloser, error)
+	// Put creates a new blob with the specified key and contents, and returns a normalized key
+	// that can be used for future R/W.
+	//
+	// Note that the returned key may be identical to that supplied in the original request;
+	// the behavior is implementation-defined.
+	Put(ctx context.Context, key string, content io.Reader) (string, error)
+	// Delete deletes a blob by key.
+	Delete(ctx context.Context, key string) error
+}

backend/internal/core/services/service_items_attachments.go

@@ -3,7 +3,6 @@ package services
 import (
 	"context"
 	"io"
-	"os"
 
 	"github.com/google/uuid"
 	"github.com/hay-kot/homebox/backend/internal/data/ent"
@@ -12,13 +11,18 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
-func (svc *ItemService) AttachmentPath(ctx context.Context, attachmentID uuid.UUID) (*ent.Document, error) {
+func (svc *ItemService) AttachmentData(ctx context.Context, attachmentID uuid.UUID) (*ent.Document, io.ReadCloser, error) {
 	attachment, err := svc.repo.Attachments.Get(ctx, attachmentID)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	return attachment.Edges.Document, nil
+	content, err := svc.repo.Docs.Read(ctx, attachment.Edges.Document.ID)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return attachment.Edges.Document, content, nil
 }
 
 func (svc *ItemService) AttachmentUpdate(ctx Context, itemID uuid.UUID, data *repo.ItemAttachmentUpdate) (repo.ItemOut, error) {
@@ -83,8 +87,8 @@ func (svc *ItemService) AttachmentDelete(ctx context.Context, gid, itemID, attac
 		return err
 	}
 
-	// Remove File
+	// Delete the document
-	err = os.Remove(attachment.Edges.Document.Path)
+	err = svc.repo.Docs.Delete(ctx, attachment.Edges.Document.ID)
 
 	return err
 }

backend/internal/core/services/service_items_attachments_test.go

@@ -2,11 +2,14 @@ package services
 
 import (
 	"context"
+	"io"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/hay-kot/homebox/backend/internal/core/blobstore"
 	"github.com/hay-kot/homebox/backend/internal/data/repo"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -15,6 +18,8 @@ import (
 func TestItemService_AddAttachment(t *testing.T) {
 	temp := os.TempDir()
 
+	bs := blobstore.NewLocalBlobStore(filepath.Join(temp, "homebox"))
+
 	svc := &ItemService{
 		repo:     tRepos,
 		filepath: temp,
@@ -53,10 +58,12 @@ func TestItemService_AddAttachment(t *testing.T) {
 	storedPath := afterAttachment.Attachments[0].Document.Path
 
 	// {root}/{group}/{item}/{attachment}
-	assert.Equal(t, path.Join(temp, "homebox", tGroup.ID.String(), "documents"), path.Dir(storedPath))
+	assert.Equal(t, path.Join(tGroup.ID.String(), "documents"), path.Dir(storedPath))
 
 	// Check that the file contents are correct
-	bts, err := os.ReadFile(storedPath)
+	bts, err := bs.Get(context.Background(), storedPath)
 	require.NoError(t, err)
-	assert.Equal(t, contents, string(bts))
+	buf, err := io.ReadAll(bts)
+	require.NoError(t, err)
+	assert.Equal(t, contents, string(buf))
 }

backend/internal/data/repo/repo_documents.go

@@ -4,21 +4,20 @@ import (
 	"context"
 	"errors"
 	"io"
-	"os"
 	"path/filepath"
 
 	"github.com/google/uuid"
+	"github.com/hay-kot/homebox/backend/internal/core/blobstore"
 	"github.com/hay-kot/homebox/backend/internal/data/ent"
 	"github.com/hay-kot/homebox/backend/internal/data/ent/document"
 	"github.com/hay-kot/homebox/backend/internal/data/ent/group"
-	"github.com/hay-kot/homebox/backend/pkgs/pathlib"
 )
 
 var ErrInvalidDocExtension = errors.New("invalid document extension")
 
 type DocumentRepository struct {
-	db  *ent.Client
+	db *ent.Client
-	dir string
+	bs blobstore.BlobStore
 }
 
 type (
@@ -47,8 +46,8 @@ var (
 	mapDocumentOutEachErr = mapTEachErrFunc(mapDocumentOut)
 )
 
-func (r *DocumentRepository) path(gid uuid.UUID, ext string) string {
+func (r *DocumentRepository) blobKey(gid uuid.UUID, ext string) string {
-	return pathlib.Safe(filepath.Join(r.dir, gid.String(), "documents", uuid.NewString()+ext))
+	return filepath.Join(gid.String(), "documents", uuid.NewString()+ext)
 }
 
 func (r *DocumentRepository) GetAll(ctx context.Context, gid uuid.UUID) ([]DocumentOut, error) {
@@ -63,26 +62,29 @@ func (r *DocumentRepository) Get(ctx context.Context, id uuid.UUID) (DocumentOut
 	return mapDocumentOutErr(r.db.Document.Get(ctx, id))
 }
 
+func (r *DocumentRepository) Read(ctx context.Context, id uuid.UUID) (io.ReadCloser, error) {
+	doc, err := r.db.Document.Get(ctx, id)
+	if err != nil {
+		return nil, err
+	}
+
+	content, err := r.bs.Get(ctx, doc.Path)
+	if err != nil {
+		return nil, err
+	}
+
+	return content, nil
+}
+
 func (r *DocumentRepository) Create(ctx context.Context, gid uuid.UUID, doc DocumentCreate) (DocumentOut, error) {
 	ext := filepath.Ext(doc.Title)
 	if ext == "" {
 		return DocumentOut{}, ErrInvalidDocExtension
 	}
 
-	path := r.path(gid, ext)
+	key := r.blobKey(gid, ext)
 
-	parent := filepath.Dir(path)
+	path, err := r.bs.Put(ctx, key, doc.Content)
-	err := os.MkdirAll(parent, 0o755)
-	if err != nil {
-		return DocumentOut{}, err
-	}
-
-	f, err := os.Create(path)
-	if err != nil {
-		return DocumentOut{}, err
-	}
-
-	_, err = io.Copy(f, doc.Content)
 	if err != nil {
 		return DocumentOut{}, err
 	}
@@ -107,7 +109,7 @@ func (r *DocumentRepository) Delete(ctx context.Context, id uuid.UUID) error {
 		return err
 	}
 
-	err = os.Remove(doc.Path)
+	err = r.bs.Delete(ctx, doc.Path)
 	if err != nil {
 		return err
 	}

backend/internal/data/repo/repo_documents_test.go

@@ -4,11 +4,12 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"os"
+	"io"
 	"path/filepath"
 	"testing"
 
 	"github.com/google/uuid"
+	"github.com/hay-kot/homebox/backend/internal/core/blobstore"
 	"github.com/hay-kot/homebox/backend/internal/data/ent"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -47,8 +48,8 @@ func useDocs(t *testing.T, num int) []DocumentOut {
 func TestDocumentRepository_CreateUpdateDelete(t *testing.T) {
 	temp := t.TempDir()
 	r := DocumentRepository{
-		db:  tClient,
+		db: tClient,
-		dir: temp,
+		bs: blobstore.NewLocalBlobStore(temp),
 	}
 
 	type args struct {
@@ -83,13 +84,15 @@ func TestDocumentRepository_CreateUpdateDelete(t *testing.T) {
 			got, err := r.Create(tt.args.ctx, tt.args.gid, tt.args.doc)
 			require.NoError(t, err)
 			assert.Equal(t, tt.title, got.Title)
-			assert.Equal(t, fmt.Sprintf("%s/%s/documents", temp, tt.args.gid), filepath.Dir(got.Path))
+			assert.Equal(t, fmt.Sprintf("%s/documents", tt.args.gid), filepath.Dir(got.Path))
 
 			ensureRead := func() {
 				// Read Document
-				bts, err := os.ReadFile(got.Path)
+				bts, err := r.bs.Get(tt.args.ctx, got.Path)
 				require.NoError(t, err)
-				assert.Equal(t, tt.content, string(bts))
+				buf, err := io.ReadAll(bts)
+				require.NoError(t, err)
+				assert.Equal(t, tt.content, string(buf))
 			}
 			ensureRead()
 
@@ -104,7 +107,7 @@ func TestDocumentRepository_CreateUpdateDelete(t *testing.T) {
 			err = r.Delete(tt.args.ctx, got.ID)
 			require.NoError(t, err)
 
-			_, err = os.Stat(got.Path)
+			_, err = r.bs.Get(tt.args.ctx, got.Path)
 			require.Error(t, err)
 		})
 	}

backend/internal/data/repo/repos_all.go

@@ -2,6 +2,7 @@
 package repo
 
 import (
+	"github.com/hay-kot/homebox/backend/internal/core/blobstore"
 	"github.com/hay-kot/homebox/backend/internal/core/services/reporting/eventbus"
 	"github.com/hay-kot/homebox/backend/internal/data/ent"
 )
@@ -28,7 +29,7 @@ func New(db *ent.Client, bus *eventbus.EventBus, root string) *AllRepos {
 		Locations:   &LocationRepository{db, bus},
 		Labels:      &LabelRepository{db, bus},
 		Items:       &ItemsRepository{db, bus},
-		Docs:        &DocumentRepository{db, root},
+		Docs:        &DocumentRepository{db, blobstore.NewLocalBlobStore(root)},
 		Attachments: &AttachmentRepo{db},
 		MaintEntry:  &MaintenanceEntryRepository{db},
 		Notifiers:   NewNotifierRepository(db),