fix(files): allow downloading issue attachments

apps/api/internal/handler/attachment_authz_test.go

@@ -0,0 +1,52 @@
+package handler_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Devlaner/devlane/api/internal/model"
+	"github.com/Devlaner/devlane/api/internal/service"
+	"github.com/Devlaner/devlane/api/internal/store"
+	"github.com/Devlaner/devlane/api/internal/testutil"
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+)
+
+// A leaked attachment object URL must not be fetchable by a non-member.
+func TestAttachment_AuthorizeDownload(t *testing.T) {
+	ts := testutil.NewTestServer(t)
+	w := testutil.SeedWorld(t, ts.DB)
+	issue := testutil.CreateIssue(t, ts.DB, w.Project.ID, w.Workspace.ID, w.User.ID)
+
+	assetID := uuid.New()
+	require.NoError(t, ts.DB.Create(&model.FileAsset{
+		ID:      assetID,
+		Asset:   "attachments/" + issue.ID.String() + "/" + assetID.String(),
+		IssueID: &issue.ID,
+	}).Error)
+	require.NoError(t, ts.DB.Create(&model.IssueAttachment{
+		ID:          uuid.New(),
+		IssueID:     issue.ID,
+		AssetID:     assetID,
+		ProjectID:   w.Project.ID,
+		WorkspaceID: w.Workspace.ID,
+	}).Error)
+
+	svc := service.NewAttachmentService(
+		store.NewIssueStore(ts.DB),
+		store.NewProjectStore(ts.DB),
+		store.NewWorkspaceStore(ts.DB),
+		nil,
+	)
+	ctx := context.Background()
+
+	// A workspace member (the owner) may download.
+	require.NoError(t, svc.AuthorizeDownload(ctx, issue.ID, assetID, w.User.ID))
+
+	// A user who isn't in the workspace is denied.
+	outsider := testutil.CreateUser(t, ts.DB)
+	require.Error(t, svc.AuthorizeDownload(ctx, issue.ID, assetID, outsider.ID))
+
+	// A non-existent attachment (wrong asset id) is not found.
+	require.Error(t, svc.AuthorizeDownload(ctx, issue.ID, uuid.New(), w.User.ID))
+}

apps/api/internal/handler/upload.go

@@ -2,6 +2,7 @@ package handler
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -11,13 +12,17 @@ import (
 
 	"github.com/Devlaner/devlane/api/internal/middleware"
 	"github.com/Devlaner/devlane/api/internal/minio"
+	"github.com/Devlaner/devlane/api/internal/service"
 	"github.com/gin-gonic/gin"
 	"github.com/google/uuid"
 )
 
 // UploadHandler handles file uploads to MinIO.
 type UploadHandler struct {
 	Minio *minio.Client
+	// Attachments authorizes downloads of attachment objects. Optional: when
+	// nil, attachment paths are refused (they can't be safely served).
+	Attachments *service.AttachmentService
 }
 
 var allowedImageTypes = map[string]bool{
@@ -100,17 +105,73 @@ func (h *UploadHandler) Upload(c *gin.Context) {
 
 // ServeFile streams a file from MinIO by path.
 // GET /api/files/*path
+// isServableObjectPath allows only the object prefixes we intend to serve —
+// generic uploads (avatars/covers/logos) and issue attachments — and rejects
+// empty paths and path traversal. Attachments live under
+// "attachments/<issueId>/<assetId>", so serving only "uploads/" made every
+// attachment download fail with 400.
+func isServableObjectPath(path string) bool {
+	if path == "" || strings.Contains(path, "..") {
+		return false
+	}
+	return strings.HasPrefix(path, "uploads/") || strings.HasPrefix(path, "attachments/")
+}
+
+// parseAttachmentPath extracts the issue and asset ids from an object path of the
+// form "attachments/<issueID>/<assetID>".
+func parseAttachmentPath(path string) (issueID, assetID uuid.UUID, ok bool) {
+	parts := strings.Split(path, "/")
+	if len(parts) != 3 || parts[0] != "attachments" {
+		return uuid.Nil, uuid.Nil, false
+	}
+	iid, err1 := uuid.Parse(parts[1])
+	aid, err2 := uuid.Parse(parts[2])
+	if err1 != nil || err2 != nil {
+		return uuid.Nil, uuid.Nil, false
+	}
+	return iid, aid, true
+}
+
 func (h *UploadHandler) ServeFile(c *gin.Context) {
 	if h.Minio == nil {
 		c.Status(http.StatusServiceUnavailable)
 		return
 	}
 	path := strings.TrimPrefix(c.Param("path"), "/")
-	if path == "" || strings.Contains(path, "..") || !strings.HasPrefix(path, "uploads/") {
+	if !isServableObjectPath(path) {
 		c.Status(http.StatusBadRequest)
 		return
 	}
 
+	// Attachment objects are per-issue, so authorize the caller against the
+	// attachment's workspace before streaming — otherwise a leaked object URL
+	// would be fetchable by anyone signed in. A 404 is returned for both missing
+	// and forbidden so we don't reveal which attachments exist.
+	if strings.HasPrefix(path, "attachments/") {
+		if h.Attachments == nil {
+			c.Status(http.StatusServiceUnavailable)
+			return
+		}
+		user := middleware.GetUser(c)
+		if user == nil {
+			c.Status(http.StatusUnauthorized)
+			return
+		}
+		issueID, assetID, ok := parseAttachmentPath(path)
+		if !ok {
+			c.Status(http.StatusBadRequest)
+			return
+		}
+		if err := h.Attachments.AuthorizeDownload(c.Request.Context(), issueID, assetID, user.ID); err != nil {
+			if errors.Is(err, service.ErrAttachmentNotFound) || errors.Is(err, service.ErrProjectForbidden) {
+				c.Status(http.StatusNotFound)
+			} else {
+				c.Status(http.StatusInternalServerError)
+			}
+			return
+		}
+	}
+
 	obj, err := h.Minio.GetObject(c.Request.Context(), path)
 	if err != nil {
 		c.Status(http.StatusNotFound)

apps/api/internal/handler/upload_path_test.go

@@ -0,0 +1,46 @@
+package handler
+
+import "testing"
+
+func TestParseAttachmentPath(t *testing.T) {
+	iid, aid, ok := parseAttachmentPath("attachments/11111111-1111-1111-1111-111111111111/22222222-2222-2222-2222-222222222222")
+	if !ok {
+		t.Fatalf("expected ok for a well-formed attachment path")
+	}
+	if iid.String() != "11111111-1111-1111-1111-111111111111" || aid.String() != "22222222-2222-2222-2222-222222222222" {
+		t.Errorf("parsed ids wrong: issue=%s asset=%s", iid, aid)
+	}
+	bad := []string{
+		"uploads/2026/07/x.png",
+		"attachments/not-a-uuid/22222222-2222-2222-2222-222222222222",
+		"attachments/11111111-1111-1111-1111-111111111111",
+		"attachments/a/b/c",
+		"attachments//",
+	}
+	for _, p := range bad {
+		if _, _, ok := parseAttachmentPath(p); ok {
+			t.Errorf("parseAttachmentPath(%q) = ok, want not-ok", p)
+		}
+	}
+}
+
+func TestIsServableObjectPath(t *testing.T) {
+	cases := []struct {
+		path string
+		want bool
+	}{
+		{"uploads/2026/07/abc.png", true},
+		{"attachments/11111111-1111-1111-1111-111111111111/22222222-2222-2222-2222-222222222222", true},
+		{"", false},
+		{"attachments/../uploads/secret", false},
+		{"uploads/../../etc/passwd", false},
+		{"secrets/key", false},
+		{"attachmentsfoo/x", false},
+		{"uploadsfoo/x", false},
+	}
+	for _, c := range cases {
+		if got := isServableObjectPath(c.path); got != c.want {
+			t.Errorf("isServableObjectPath(%q) = %v, want %v", c.path, got, c.want)
+		}
+	}
+}

apps/api/internal/router/router.go

@@ -264,7 +264,7 @@ func New(cfg Config) *gin.Engine {
 		api.POST("/instance/admins/", instanceSettingsHandler.AddAdmin)
 		api.DELETE("/instance/admins/:id/", instanceSettingsHandler.RemoveAdmin)
 
-		uploadHandler := &handler.UploadHandler{Minio: cfg.Minio}
+		uploadHandler := &handler.UploadHandler{Minio: cfg.Minio, Attachments: attachmentSvc}
 		api.POST("/upload", uploadHandler.Upload)
 		api.GET("/files/*path", uploadHandler.ServeFile)
 		api.GET("/users/me/workspaces/", workspaceHandler.List)

apps/api/internal/service/attachment.go

@@ -84,6 +84,28 @@ func (s *AttachmentService) ensureIssueAccess(ctx context.Context, workspaceSlug
 	return nil
 }
 
+// AuthorizeDownload checks that userID may fetch the attachment identified by an
+// object path "attachments/<issueID>/<assetID>". It resolves the attachment
+// record (404 if missing) and requires the caller to be a member of the
+// attachment's workspace, so a leaked object URL can't be fetched by outsiders.
+func (s *AttachmentService) AuthorizeDownload(ctx context.Context, issueID, assetID, userID uuid.UUID) error {
+	att, err := s.is.GetAttachmentByAssetID(ctx, assetID, issueID)
+	if errors.Is(err, gorm.ErrRecordNotFound) || (err == nil && att == nil) {
+		return ErrAttachmentNotFound
+	}
+	if err != nil {
+		return err // a real datastore failure — surface it as 5xx, not a 404
+	}
+	ok, err := s.ws.IsMember(ctx, att.WorkspaceID, userID)
+	if err != nil {
+		return err
+	}
+	if !ok {
+		return ErrProjectForbidden
+	}
+	return nil
+}
+
 // InitiateUpload creates the DB records and returns the presigned upload URL + attachment shape.
 func (s *AttachmentService) InitiateUpload(ctx context.Context, workspaceSlug string, projectID, issueID uuid.UUID, userID uuid.UUID, name string, size float64, contentType string) (*PresignedUploadResponse, error) {
 	if s.minio == nil {