highperfocused/multica
1
0
Fork 0
mirror of https://github.com/multica-ai/multica.git synced 2026-06-17 03:38:32 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(storage): scope S3 upload keys by workspace (#936)

Browse Source 
* fix(storage): scope S3 upload keys by workspace

Upload keys now use `workspaces/{workspace_id}/{uuid}.{ext}` instead of
flat `{uuid}.{ext}`, isolating file storage per workspace. Files uploaded
without workspace context (e.g. avatars) keep the flat key structure.

Refs: MUL-577

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(storage): scope user uploads under users/{user_id}/ prefix

Non-workspace uploads (avatars, profile images) now use
`users/{user_id}/{uuid}.{ext}` instead of flat `{uuid}.{ext}`,
matching the workspace-scoped pattern from the previous commit.

Refs: MUL-577

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(storage): fix LocalStorage for nested key paths

- Add MkdirAll before WriteFile to create intermediate directories
  for workspace/user-scoped keys
- Fix KeyFromURL to preserve full path after /uploads/ prefix instead
  of stripping to just the filename
- Update tests to match new behavior

Refs: MUL-577

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(upload): validate ownership before writing to storage

Move Storage.Upload after issue_id/comment_id ownership validation
to prevent orphaned files in S3 when validation fails. Previously,
the file was uploaded first and validation happened after, leaving
files in workspace-scoped S3 prefixes even on rejected requests.

Refs: MUL-577

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(upload): restore workspace membership check before upload

The membership check was accidentally removed during the upload
reordering refactor. Without it, any authenticated user could upload
files to any workspace by setting the X-Workspace-ID header.

Also restores the comment explaining the 200-on-DB-error behavior.

Refs: MUL-577

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Devv <devv@Devvs-Mac-mini.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
devv-eve
2026-04-13 21:01:50 -07:00
committed by GitHub
parent 8c0708bb5d
commit 7c7d7feed3
3 changed files with 36 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
server/internal/handler/file.go
View File

@@ -160,16 +160,15 @@ func (h *Handler) UploadFile(w http.ResponseWriter, r *http.Request) {
writeError(w, http.StatusInternalServerError, "internal error")
return
}
key := id.String() + path.Ext(header.Filename)
link, err := h.Storage.Upload(r.Context(), key, data, contentType, header.Filename)
if err != nil {
slog.Error("file upload failed", "error", err)
writeError(w, http.StatusInternalServerError, "upload failed")
return
filename := id.String() + path.Ext(header.Filename)
var key string
if workspaceID != "" {
key = "workspaces/" + workspaceID + "/" + filename
} else {
key = "users/" + userID + "/" + filename
}
// If workspace context is available, create an attachment record.
// If workspace context is available, validate membership before uploading.
if workspaceID != "" {
if _, err := h.getWorkspaceMember(r.Context(), userID, workspaceID); err != nil {
writeError(w, http.StatusForbidden, "not a member of this workspace")
@@ -184,12 +183,10 @@ func (h *Handler) UploadFile(w http.ResponseWriter, r *http.Request) {
UploaderType: uploaderType,
UploaderID: parseUUID(uploaderID),
Filename: header.Filename,
Url: link,
ContentType: contentType,
SizeBytes: int64(len(data)),
}
// Optional issue_id / comment_id from form fields — validate ownership.
if issueID := r.FormValue("issue_id"); issueID != "" {
issue, err := h.Queries.GetIssueInWorkspace(r.Context(), db.GetIssueInWorkspaceParams{
ID: parseUUID(issueID),
@@ -210,6 +207,14 @@ func (h *Handler) UploadFile(w http.ResponseWriter, r *http.Request) {
params.CommentID = comment.ID
}
link, err := h.Storage.Upload(r.Context(), key, data, contentType, header.Filename)
if err != nil {
slog.Error("file upload failed", "error", err)
writeError(w, http.StatusInternalServerError, "upload failed")
return
}
params.Url = link
att, err := h.Queries.CreateAttachment(r.Context(), params)
if err != nil {
slog.Error("failed to create attachment record", "error", err)
@@ -219,9 +224,21 @@ func (h *Handler) UploadFile(w http.ResponseWriter, r *http.Request) {
writeJSON(w, http.StatusOK, h.attachmentToResponse(att))
return
}
writeJSON(w, http.StatusOK, map[string]string{
"filename": header.Filename,
"link": link,
})
return
}
// Fallback response (no workspace context, e.g. avatar upload)
// No workspace context (e.g. avatar upload) — upload directly.
link, err := h.Storage.Upload(r.Context(), key, data, contentType, header.Filename)
if err != nil {
slog.Error("file upload failed", "error", err)
writeError(w, http.StatusInternalServerError, "upload failed")
return
}
writeJSON(w, http.StatusOK, map[string]string{
"filename": header.Filename,
"link": link,

11
server/internal/storage/local.go
View File

@@ -48,12 +48,8 @@ func (s *LocalStorage) KeyFromURL(rawURL string) string {
}
prefix := "/uploads/"
if strings.HasPrefix(rawURL, prefix) {
filename := strings.TrimPrefix(rawURL, prefix)
if i := strings.LastIndex(filename, "/"); i >= 0 {
return filename[i+1:]
}
return filename
if idx := strings.Index(rawURL, prefix); idx >= 0 {
return rawURL[idx+len(prefix):]
}
if i := strings.LastIndex(rawURL, "/"); i >= 0 {
return rawURL[i+1:]
@@ -81,6 +77,9 @@ func (s *LocalStorage) DeleteKeys(ctx context.Context, keys []string) {
func (s *LocalStorage) Upload(ctx context.Context, key string, data []byte, contentType string, filename string) (string, error) {
dest := filepath.Join(s.uploadDir, key)
if err := os.MkdirAll(filepath.Dir(dest), 0755); err != nil {
return "", fmt.Errorf("local storage MkdirAll: %w", err)
}
if err := os.WriteFile(dest, data, 0644); err != nil {
return "", fmt.Errorf("local storage WriteFile: %w", err)
}

5
server/internal/storage/local_test.go
View File

@@ -124,7 +124,8 @@ func TestLocalStorage_KeyFromURL(t *testing.T) {
expected string
}{
{"local URL format", "/uploads/abc123.png", "abc123.png"},
{"local URL with subdir", "/uploads/2024/01/image.jpg", "image.jpg"},
{"local URL with subdir", "/uploads/2024/01/image.jpg", "2024/01/image.jpg"},
{"local URL with workspace prefix", "/uploads/workspaces/ws-123/abc.png", "workspaces/ws-123/abc.png"},
{"just filename", "abc123.png", "abc123.png"},
{"full path", "/some/path/to/file.pdf", "file.pdf"},
}
@@ -155,7 +156,7 @@ func TestLocalStorage_KeyFromURL_WithBaseURL(t *testing.T) {
expected string
}{
{"full URL format", "http://localhost:8080/uploads/abc123.png", "abc123.png"},
{"full URL with subdir", "http://localhost:8080/uploads/2024/01/image.jpg", "image.jpg"},
{"full URL with subdir", "http://localhost:8080/uploads/2024/01/image.jpg", "2024/01/image.jpg"},
{"local URL format still works", "/uploads/abc123.png", "abc123.png"},
}