feat(artifact)!: introduce blob store abstraction with logging and security improvements

Adds BlobStore interface separating artifact metadata persistence from filesystem operations. Includes path traversal protection in local FS, connection pooling fixes, MIME detection refactoring, and structured logging wrappers for debugging cloud storage operations.

BREAKING CHANGE: introduce blob store abstraction with logging and security improvements

Author: moshloop

artifact/blob_store.go

@@ -0,0 +1,107 @@
+package artifact
+
+import (
+	"bytes"
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
+	"io"
+
+	"github.com/flanksource/duty/models"
+	"github.com/google/uuid"
+	"gorm.io/gorm"
+)
+
+type BlobStore interface {
+	Write(data Data, artifact *models.Artifact) (*models.Artifact, error)
+	Read(artifactID uuid.UUID) (*Data, error)
+	io.Closer
+}
+
+type blobStore struct {
+	fs      FilesystemRW
+	db      *gorm.DB
+	backend string
+}
+
+func NewBlobStore(fs FilesystemRW, db *gorm.DB, backend string) BlobStore {
+	return &blobStore{fs: fs, db: db, backend: backend}
+}
+
+func (s *blobStore) Write(data Data, a *models.Artifact) (*models.Artifact, error) {
+	if data.Content == nil {
+		return nil, fmt.Errorf("artifact data content is nil")
+	}
+	defer func() { _ = data.Content.Close() }()
+
+	checksum := sha256.New()
+	mimeReader := io.TeeReader(data.Content, checksum)
+
+	mw := &mimeWriter{Max: maxBytesForMimeDetection}
+	fileReader := io.TeeReader(mimeReader, mw)
+
+	info, err := s.fs.Write(s.db.Statement.Context, data.Filename, fileReader)
+	if err != nil {
+		return nil, fmt.Errorf("writing artifact %s: %w", data.Filename, err)
+	}
+
+	if data.ContentType == "" {
+		data.ContentType = mw.Detect().String()
+	}
+
+	// For inline store, the artifact already has content set
+	if inlineArt := InlineArtifact(info); inlineArt != nil {
+		a.Content = inlineArt.Content
+		a.CompressionType = inlineArt.CompressionType
+	}
+
+	a.Path = data.Filename
+	a.Filename = info.Name()
+	a.Size = info.Size()
+	a.ContentType = data.ContentType
+	a.Checksum = hex.EncodeToString(checksum.Sum(nil))
+
+	if err := s.db.Create(a).Error; err != nil {
+		return nil, fmt.Errorf("saving artifact to db: %w", err)
+	}
+
+	return a, nil
+}
+
+func (s *blobStore) Read(artifactID uuid.UUID) (*Data, error) {
+	var a models.Artifact
+	if err := s.db.Where("id = ?", artifactID).First(&a).Error; err != nil {
+		return nil, fmt.Errorf("finding artifact %s: %w", artifactID, err)
+	}
+
+	if a.IsInline() {
+		content, err := a.GetContent()
+		if err != nil {
+			return nil, fmt.Errorf("decompressing inline artifact %s: %w", artifactID, err)
+		}
+		return &Data{
+			Content:       io.NopCloser(bytes.NewReader(content)),
+			ContentLength: a.Size,
+			Checksum:      a.Checksum,
+			ContentType:   a.ContentType,
+			Filename:      a.Filename,
+		}, nil
+	}
+
+	r, err := s.fs.Read(s.db.Statement.Context, a.Path)
+	if err != nil {
+		return nil, fmt.Errorf("reading artifact %s from %s: %w", artifactID, a.Path, err)
+	}
+
+	return &Data{
+		Content:       r,
+		ContentLength: a.Size,
+		Checksum:      a.Checksum,
+		ContentType:   a.ContentType,
+		Filename:      a.Filename,
+	}, nil
+}
+
+func (s *blobStore) Close() error {
+	return s.fs.Close()
+}

artifact/clients/aws/fileinfo.go

@@ -17,6 +17,9 @@ type S3FileInfo struct {
 }
 
 func (obj S3FileInfo) Name() string {
+	if obj.Object.Key == nil {
+		return ""
+	}
 	return *obj.Object.Key
 }
 

artifact/clients/sftp/sftp.go

@@ -5,13 +5,16 @@ import (
 	"golang.org/x/crypto/ssh"
 )
 
+// SSHConnect creates an SFTP client connection.
+// NOTE: Uses InsecureIgnoreHostKey because artifact storage targets are
+// configured by admins via trusted connection objects, not user input.
 func SSHConnect(host, user, password string) (*sftp.Client, error) {
 	config := &ssh.ClientConfig{
 		User: user,
 		Auth: []ssh.AuthMethod{
 			ssh.Password(password),
 		},
-		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
+		HostKeyCallback: ssh.InsecureIgnoreHostKey(), //nolint:gosec
 	}
 
 	conn, err := ssh.Dial("tcp", host, config)
@@ -21,6 +24,7 @@ func SSHConnect(host, user, password string) (*sftp.Client, error) {
 
 	client, err := sftp.NewClient(conn)
 	if err != nil {
+		conn.Close()
 		return nil, err
 	}
 

artifact/clients/smb/smb.go

@@ -35,11 +35,14 @@ func SMBConnect(server string, port, share string, auth types.Authentication) (*
 
 	s, err := d.Dial(conn)
 	if err != nil {
+		conn.Close()
 		return nil, err
 	}
 	smb.Session = s
 	fs, err := s.Mount(share)
 	if err != nil {
+		_ = s.Logoff()
+		conn.Close()
 		return nil, err
 	}
 

artifact/data.go

@@ -0,0 +1,69 @@
+package artifact
+
+import (
+	"fmt"
+	"io"
+
+	"github.com/flanksource/clicky"
+	"github.com/flanksource/clicky/api"
+	"github.com/gabriel-vasile/mimetype"
+)
+
+type Data struct {
+	Content       io.ReadCloser
+	ContentLength int64
+	Checksum      string
+	ContentType   string
+	Filename      string
+}
+
+func (d Data) Pretty() api.Text {
+	s := clicky.Text(d.Filename, "font-bold")
+	if d.ContentType != "" {
+		s = s.AddText(" "+d.ContentType, "text-gray-500")
+	}
+	if d.ContentLength > 0 {
+		s = s.AddText(fmt.Sprintf(" (%s)", formatBytes(d.ContentLength)), "text-gray-400")
+	}
+	if d.Checksum != "" {
+		short := d.Checksum
+		if len(short) > 8 {
+			short = short[:8]
+		}
+		s = s.AddText(" sha:"+short, "text-yellow-500")
+	}
+	return s
+}
+
+func formatBytes(b int64) string {
+	switch {
+	case b >= 1<<20:
+		return fmt.Sprintf("%.1f MB", float64(b)/(1<<20))
+	case b >= 1<<10:
+		return fmt.Sprintf("%.1f KB", float64(b)/(1<<10))
+	default:
+		return fmt.Sprintf("%d B", b)
+	}
+}
+
+const maxBytesForMimeDetection = 512 * 1024
+
+type mimeWriter struct {
+	buffer []byte
+	Max    int
+}
+
+func (t *mimeWriter) Write(bb []byte) (n int, err error) {
+	if len(t.buffer) < t.Max {
+		rem := t.Max - len(t.buffer)
+		if rem > len(bb) {
+			rem = len(bb)
+		}
+		t.buffer = append(t.buffer, bb[:rem]...)
+	}
+	return len(bb), nil
+}
+
+func (t *mimeWriter) Detect() *mimetype.MIME {
+	return mimetype.Detect(t.buffer)
+}

artifact/fs/local.go

@@ -12,6 +12,22 @@ import (
 	"github.com/flanksource/duty/artifact"
 )
 
+func (t *localFS) safePath(path string) (string, error) {
+	full := filepath.Join(t.base, path)
+	abs, err := filepath.Abs(full)
+	if err != nil {
+		return "", fmt.Errorf("resolving path: %w", err)
+	}
+	baseAbs, err := filepath.Abs(t.base)
+	if err != nil {
+		return "", fmt.Errorf("resolving base: %w", err)
+	}
+	if !strings.HasPrefix(abs, baseAbs) {
+		return "", fmt.Errorf("path %q escapes base directory", path)
+	}
+	return full, nil
+}
+
 type localFS struct {
 	base string
 }
@@ -80,28 +96,38 @@ func (t *localFS) ReadDirGlob(name string) ([]artifact.FileInfo, error) {
 }
 
 func (t *localFS) Stat(name string) (os.FileInfo, error) {
-	return os.Stat(filepath.Join(t.base, name))
+	p, err := t.safePath(name)
+	if err != nil {
+		return nil, err
+	}
+	return os.Stat(p)
 }
 
 func (t *localFS) Read(_ gocontext.Context, path string) (io.ReadCloser, error) {
-	return os.Open(filepath.Join(t.base, path))
+	p, err := t.safePath(path)
+	if err != nil {
+		return nil, err
+	}
+	return os.Open(p)
 }
 
 func (t *localFS) Write(_ gocontext.Context, path string, data io.Reader) (os.FileInfo, error) {
-	fullpath := filepath.Join(t.base, path)
-
-	err := os.MkdirAll(filepath.Dir(fullpath), os.ModePerm)
+	fullpath, err := t.safePath(path)
 	if err != nil {
+		return nil, err
+	}
+
+	if err := os.MkdirAll(filepath.Dir(fullpath), os.ModePerm); err != nil {
 		return nil, fmt.Errorf("error creating base directory: %w", err)
 	}
 
 	f, err := os.Create(fullpath)
 	if err != nil {
 		return nil, err
 	}
+	defer f.Close()
 
-	_, err = io.Copy(f, data)
-	if err != nil {
+	if _, err = io.Copy(f, data); err != nil {
 		return nil, err
 	}
 

artifact/fs/save.go

@@ -18,16 +18,14 @@ type MIMEWriter struct {
 }
 
 func (t *MIMEWriter) Write(bb []byte) (n int, err error) {
-	if len(t.buffer) > t.Max {
-		return 0, nil
+	if len(t.buffer) < t.Max {
+		rem := t.Max - len(t.buffer)
+		if rem > len(bb) {
+			rem = len(bb)
+		}
+		t.buffer = append(t.buffer, bb[:rem]...)
 	}
-
-	rem := t.Max - len(t.buffer)
-	if rem > len(bb) {
-		rem = len(bb)
-	}
-	t.buffer = append(t.buffer, bb[:rem]...)
-	return rem, nil
+	return len(bb), nil
 }
 
 func (t *MIMEWriter) Detect() *mimetype.MIME {

artifact/fs/smb.go

@@ -41,6 +41,10 @@ func NewSMBFS(server string, port, share string, auth types.Authentication) (*sm
 	return &smbFS{SMBSession: session}, nil
 }
 
+func (s *smbFS) Close() error {
+	return s.SMBSession.Close()
+}
+
 func (s *smbFS) Read(_ gocontext.Context, path string) (io.ReadCloser, error) {
 	return s.Open(path)
 }
@@ -50,6 +54,7 @@ func (s *smbFS) Write(_ gocontext.Context, path string, data io.Reader) (os.File
 	if err != nil {
 		return nil, err
 	}
+	defer f.Close()
 
 	if _, err = io.Copy(f, data); err != nil {
 		return nil, fmt.Errorf("error writing file: %w", err)
@@ -90,7 +95,7 @@ func (t *smbFS) ReadDirGlob(name string) ([]artifact.FileInfo, error) {
 		if err != nil {
 			return nil, err
 		}
-		output = append(output, &SMBFileInfo{Base: name, FileInfo: info})
+		output = append(output, &SMBFileInfo{Base: base, FileInfo: info})
 	}
 
 	return output, nil

artifact/fs/ssh.go

@@ -89,6 +89,10 @@ func (s *sshFS) Read(_ gocontext.Context, path string) (io.ReadCloser, error) {
 	return s.Open(path)
 }
 
+func (s *sshFS) Close() error {
+	return s.Client.Close()
+}
+
 func (s *sshFS) Write(_ gocontext.Context, path string, data io.Reader) (os.FileInfo, error) {
 	dir := filepath.Dir(path)
 	if err := s.MkdirAll(dir); err != nil {
@@ -99,6 +103,7 @@ func (s *sshFS) Write(_ gocontext.Context, path string, data io.Reader) (os.File
 	if err != nil {
 		return nil, fmt.Errorf("error creating file: %w", err)
 	}
+	defer f.Close()
 
 	if _, err = io.Copy(f, data); err != nil {
 		return nil, fmt.Errorf("error writing to file: %w", err)