From 504d348917c2fb8ed2c139bbbc352e07fccae19a Mon Sep 17 00:00:00 2001 From: xarmian Date: Wed, 29 Apr 2026 17:44:12 -0400 Subject: [PATCH] =?UTF-8?q?feat(attachments):=20Settings=20=E2=86=92=20Sto?= =?UTF-8?q?rage=20tab=20with=20attachment=20list=20(TASK-882)=20(#303)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(attachments): Settings → Storage tab with attachment list (TASK-882) Adds the Settings → Storage tab and the underlying list/delete API endpoints so workspace owners can audit and reclaim attachment bytes. Backend (TASK-882 needs this — there was no list/delete API yet): - store.WorkspaceAttachments: paginated list with filter (category, attached/unattached, collection_id) + sort allowlist (size, filename, created_at — each with desc variant). LEFT JOIN to items + collections enriches each row with item_title/slug + collection_slug for the "in [[Item]]" link. Hides derived (thumbnail) rows by default — they count toward quota but are managed automatically and would clutter the user-facing list. - store.SoftDeleteAttachment: tombstones the row + every variant. Blob on disk stays put; orphan GC reclaims past the grace period (TASK-886). - GET /workspaces/{ws}/attachments — viewer+, returns {attachments, total, limit, offset}. - DELETE /workspaces/{ws}/attachments/{id} — editor+. Refuses to delete derived rows directly (returns 400 with derived_attachment code) and invalidates the storage-usage cache. Frontend: - StorageTab.svelte component (lib/components/settings) with usage bar (color thresholds at 80%/100%, override badge), 5-select filter row (category, item, collection, sort, page size), attachment list with thumbnails (image variants via thumb-sm, emoji icon otherwise), item link, MIME, size, date, and per-row delete with confirm() dialog. Pagination footer with Prev/Next + "showing X–Y of Z". - TS api.attachments.list() / delete() + types. - Wired as a new "Storage" tab on the workspace settings page. Tests: - TestListAttachments_Pagination: 3 uploads, default + size-asc sort, limit/offset paging. - TestListAttachments_HidesDerived: synthetic thumbnail row, asserts the list excludes parent_id != NULL rows. - TestDeleteAttachment_HappyPath: upload → delete → list empty → storage usage drops to 0 (cache invalidation hook fires) → second delete returns 404. - TestDeleteAttachment_DerivedRefused: thumbnail rows can't be deleted directly via the API. Parent: PLAN-866. * fix(attachments): collection visibility + category gaps + item ref shape per Codex (round 1) Three findings from Codex on PR #303 round 1: P1 — Collection visibility leak. The storage list returned all workspace attachments without applying per-user collection access, so a member with collection_access=specific would receive hidden collections' attachment IDs/filenames/item titles and could then pull the bytes via the existing download endpoint. Fixed by threading visibleCollectionIDs(r, workspaceID) through to the store filter. nil = admin/no restriction; empty slice = zero visible collections (zero rows by design); explicit set = restrict i.collection_id IN (...). Orphans (item_id IS NULL) are excluded for restricted users since their filenames would still leak. P2 — item_ref shape didn't match the route. The store synthesized "/" and the UI inserted it verbatim into the URL, producing /user/ws/tasks/tasks/5. Dropped item_ref entirely; UI now builds URLs from item_slug + collection_slug which is the actual route shape. P2 — Category filter coverage. mimePrefixForCategory only handled image/video/audio. Selecting Documents/Text/Archive/Other in the UI silently passed through with no MIME predicate so the list showed everything. Replaced with mimePredicateForCategory which emits the right SQL fragment per bucket: prefix LIKE for the type/ buckets, explicit IN list for document/text/archive (mirroring the allowlist in internal/attachments/mime.go), and a NOT-IN composite for "other". Tests: - TestWorkspaceAttachments_VisibilityFilter: admin sees all 3 rows; restricted to one collection sees only that collection's row + orphan suppressed; empty visibility yields zero rows. - TestWorkspaceAttachments_CategoryFilters: image/document/text/ archive/other each return exactly the matching MIME types. * fix(attachments): item-level visibility on list + delete per Codex (round 2) Two more findings from Codex on PR #303 round 2: 1. The list filter used VisibleCollectionIDs alone — but that set includes collections containing any item-level grant for the user. A guest with one item granted in collection B would still receive attachment metadata for every item in collection B. Replaced with the (fullCollIDs, grantedItemIDs) tuple from guestResourceFilter so the SQL ORs collection-level full access against per-item grants, matching how handlers_search / handlers_activity narrow lists. 2. The delete endpoint validated workspace membership but never checked the attachment's parent item is visible to the caller. An editor with restricted collection access could delete attachments in hidden collections by guessing/obtaining the attachment ID. Added requireItemVisible after fetching the parent item, plus a fallback gate for orphan attachments (item_id IS NULL) so restricted users get 404 there as well. Store-level filter renamed: VisibleCollectionIDs → Restricted + FullCollectionIDs + GrantedItemIDs. Tests cover the collection-only, item-grant-only, and zero-visibility paths. * fix(attachments): allow deleting attachments when parent item is soft-deleted (round 3) Codex P2 from PR #303 round 3: the storage list intentionally surfaces attachments whose parent item has been soft-deleted (so the user sees what's still consuming quota), but the delete handler used GetItem, which filters soft-deleted out and returned 404 before SoftDeleteAttachment could run — turning every Delete button on those rows into a no-op. Fixed by adding store.GetItemIncludeDeleted (mirroring the existing GetItemBySlugIncludeDeleted) and switching the delete path to use it. The visibility check still keys off the (still-set) collection_id, so soft-deleting an item doesn't escalate access — restricted users still hit requireItemVisible's 404 if they couldn't see the parent. Regression test: create item → attach → soft-delete item → list still returns the row → delete returns 204. * fix(attachments): list surfaces attachments under soft-deleted parents (round 4) Codex round-4 finding: WorkspaceAttachments still LEFT JOIN'd items with AND i.deleted_at IS NULL, so attachments whose parent item was soft-deleted disappeared from the list — even though the previous round wired GetItemIncludeDeleted on the delete path. Net effect: restricted editors with access to that collection couldn't discover the row in the UI; only full-access users saw it as an orphan-looking entry. Fix: drop the deleted_at filter from the JOIN. The collection ACL predicate (i.collection_id IN ...) now sees the (still-set) collection_id from the soft-deleted item, so visibility behaves consistently for live and tombstoned parents. Soft-deleted items don't escalate access — the collection_id stays put. UX: response now carries item_deleted=true when the parent is soft-deleted; the StorageTab renders the title with strike-through + a small "deleted" badge instead of a clickable link (which would 404). Tests: - store-level: admin/full-access sees the row + ItemDeleted flag, restricted-to-correct-collection sees it, restricted-to-other- collection does not. - (existing TestDeleteAttachment_AfterParentSoftDeleted continues to pass on the handler side.) --- internal/server/handlers_storage.go | 231 ++++++ internal/server/handlers_storage_test.go | 320 ++++++++ internal/server/server.go | 2 + internal/store/attachments.go | 427 +++++++++++ internal/store/attachments_test.go | 280 +++++++ internal/store/items.go | 52 ++ web/src/lib/api/client.ts | 47 +- .../lib/components/settings/StorageTab.svelte | 701 ++++++++++++++++++ web/src/lib/types/index.ts | 62 ++ .../[workspace]/settings/+page.svelte | 6 + 10 files changed, 2127 insertions(+), 1 deletion(-) create mode 100644 web/src/lib/components/settings/StorageTab.svelte diff --git a/internal/server/handlers_storage.go b/internal/server/handlers_storage.go index 032950e6..f870ae0d 100644 --- a/internal/server/handlers_storage.go +++ b/internal/server/handlers_storage.go @@ -1,10 +1,16 @@ package server import ( + "database/sql" + "errors" "net/http" + "strconv" + "strings" "sync" "time" + "github.com/go-chi/chi/v5" + "github.com/PerpetualSoftware/pad/internal/store" ) @@ -129,3 +135,228 @@ func (s *Server) handleGetWorkspaceStorageUsage(w http.ResponseWriter, r *http.R s.storageInfoCache.set(workspaceID, info) writeJSON(w, http.StatusOK, info) } + +// handleListWorkspaceAttachments returns a paginated list of original +// (non-derived) attachments in the workspace. Supports filter + +// sort + pagination via query string: +// +// GET /api/v1/workspaces/{ws}/attachments +// ?category=image|video|audio|document|text|archive|other +// &item=attached|unattached +// &collection= +// &sort=size|size_desc|filename|filename_desc|created_at|created_at_desc +// &limit=<1..200> +// &offset= +// +// Unknown values are silently ignored — the server defaults +// (`created_at_desc`, limit 50, offset 0, no filters) take over. +// +// Auth: viewer+. Same gate as storage/usage — workspace-wide +// attachment metadata leaks the same surface area. +// +// Response: {attachments: [...], total: N, limit, offset}. +func (s *Server) handleListWorkspaceAttachments(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "viewer") { + return + } + workspaceID, ok := s.getWorkspaceID(w, r) + if !ok { + return + } + + q := r.URL.Query() + filters := store.AttachmentListFilters{ + MimeCategory: strings.ToLower(strings.TrimSpace(q.Get("category"))), + CollectionID: strings.TrimSpace(q.Get("collection")), + Sort: strings.ToLower(strings.TrimSpace(q.Get("sort"))), + } + switch strings.ToLower(strings.TrimSpace(q.Get("item"))) { + case "attached": + filters.Attached = true + case "unattached": + filters.Unattached = true + } + if v := q.Get("limit"); v != "" { + if n, err := strconv.Atoi(v); err == nil { + filters.Limit = n + } + } + if v := q.Get("offset"); v != "" { + if n, err := strconv.Atoi(v); err == nil { + filters.Offset = n + } + } + + // Enforce per-user collection + item-level access control. Mirrors + // the (fullCollIDs, grantedItemIDs) tuple used by handlers_search / + // handlers_activity for cross-collection lists. nil/nil from + // guestResourceFilter means admin or full-access member — no + // restriction. Otherwise a restricted user's view is the union of + // their member-access collections + any item-level grants. + fullCollIDs, grantedItemIDs, err := s.guestResourceFilter(r, workspaceID) + if err != nil { + writeInternalError(w, err) + return + } + restricted := fullCollIDs != nil || grantedItemIDs != nil + if restricted { + filters.Restricted = true + filters.FullCollectionIDs = fullCollIDs + filters.GrantedItemIDs = grantedItemIDs + // Restricted users never see orphans (item_id IS NULL) — the + // store filter already excludes them, so the Unattached + // filter would always yield zero rows. Short-circuit so the + // UI sees an immediate empty page rather than firing the SQL. + if filters.Unattached { + writeJSON(w, http.StatusOK, map[string]any{ + "attachments": []store.AttachmentListItem{}, + "total": 0, + "limit": effectiveLimit(filters.Limit), + "offset": effectiveOffset(filters.Offset), + }) + return + } + } + + rows, total, err := s.store.WorkspaceAttachments(workspaceID, filters) + if err != nil { + writeInternalError(w, err) + return + } + if rows == nil { + // Marshal `[]` rather than `null` so the UI can iterate + // without a falsy-check guard on every render. + rows = []store.AttachmentListItem{} + } + + writeJSON(w, http.StatusOK, map[string]any{ + "attachments": rows, + "total": total, + "limit": effectiveLimit(filters.Limit), + "offset": effectiveOffset(filters.Offset), + }) +} + +// effectiveLimit / effectiveOffset mirror the store-side defaults so +// the response carries the canonical values the handler used. Useful +// to the UI when the request omitted them. +func effectiveLimit(n int) int { + if n <= 0 { + return 50 + } + if n > 200 { + return 200 + } + return n +} + +func effectiveOffset(n int) int { + if n < 0 { + return 0 + } + return n +} + +// handleDeleteWorkspaceAttachment soft-deletes an attachment by ID. +// Tombstones the row + every thumbnail variant; the orphan GC reclaims +// the on-disk blob after the grace period (TASK-886). +// +// DELETE /api/v1/workspaces/{ws}/attachments/{attachmentID} +// +// Auth: editor+. Delete is destructive (the bytes go away after GC) — +// view-only members shouldn't be able to remove attachments other +// users uploaded. +// +// Cross-workspace requests get 404 (not 403) to avoid leaking which +// IDs exist in other workspaces. Same pattern as the download +// handler. +// +// Returns 204 on success. The storage-usage cache is invalidated +// eagerly so the bar drops within a refresh cycle. +func (s *Server) handleDeleteWorkspaceAttachment(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } + workspaceID, ok := s.getWorkspaceID(w, r) + if !ok { + return + } + id := chi.URLParam(r, "attachmentID") + if id == "" { + writeError(w, http.StatusBadRequest, "missing_id", "Attachment ID required") + return + } + + att, err := s.store.GetAttachment(id) + if err != nil { + writeInternalError(w, err) + return + } + if att == nil || att.WorkspaceID != workspaceID || att.DeletedAt != nil { + writeError(w, http.StatusNotFound, "not_found", "Attachment not found") + return + } + // Refuse to delete derived (thumbnail) rows directly — they're + // auto-managed and deleting the original cascades. A direct + // delete here would leave the original without thumbnails until + // a future "regenerate" job runs. + if att.ParentID != nil { + writeError(w, http.StatusBadRequest, "derived_attachment", + "Cannot delete a thumbnail directly — delete the original.") + return + } + + // Item-level visibility check. An editor with restricted collection + // access shouldn't be able to delete attachments in collections + // they can't see — even if they obtain the attachment ID some + // other way. Mirrors requireItemVisible's logic but operates on + // the attachment's parent item id rather than a fully-loaded item. + // + // GetItemIncludeDeleted is used (not GetItem) because the storage + // list intentionally surfaces attachments whose parent item is + // soft-deleted — they're still consuming quota and the user + // needs a path to delete the blob. The collection_id stays set + // after a soft delete, so the visibility predicate still works. + if att.ItemID != nil { + item, err := s.store.GetItemIncludeDeleted(*att.ItemID) + if err != nil { + writeInternalError(w, err) + return + } + if item == nil || !s.requireItemVisible(w, r, workspaceID, item) { + // requireItemVisible already wrote a 404 on its denial path. + // Two cases land us here: the item was hard-deleted out from + // under us (item == nil) or the user can't see it. + if item == nil { + writeError(w, http.StatusNotFound, "not_found", "Attachment not found") + } + return + } + } else { + // Orphan attachments (item_id IS NULL) are not associated with + // any collection, so collection-level visibility doesn't apply. + // Restricted members shouldn't reach here because the LIST + // endpoint hides orphans from them, but a direct DELETE with a + // guessed UUID could — gate orphans on full-access editors. + if fullCollIDs, grantedItemIDs, gErr := s.guestResourceFilter(r, workspaceID); gErr != nil { + writeInternalError(w, gErr) + return + } else if fullCollIDs != nil || grantedItemIDs != nil { + writeError(w, http.StatusNotFound, "not_found", "Attachment not found") + return + } + } + + if err := s.store.SoftDeleteAttachment(id); err != nil { + if errors.Is(err, sql.ErrNoRows) { + writeError(w, http.StatusNotFound, "not_found", "Attachment not found") + return + } + writeInternalError(w, err) + return + } + + s.storageInfoCache.invalidate(workspaceID) + + w.WriteHeader(http.StatusNoContent) +} diff --git a/internal/server/handlers_storage_test.go b/internal/server/handlers_storage_test.go index c415294f..b3cd18cd 100644 --- a/internal/server/handlers_storage_test.go +++ b/internal/server/handlers_storage_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/PerpetualSoftware/pad/internal/models" "github.com/PerpetualSoftware/pad/internal/store" ) @@ -114,6 +115,325 @@ func TestStorageUsage_RejectsGuests(t *testing.T) { } } +// TestListAttachments_Pagination covers list + paginate + sort ordering. +// Uploads three blobs of distinct sizes and verifies: +// - default sort is created_at DESC (newest first) +// - sort=size ascends from smallest +// - limit + offset paginate correctly with the right total count +func TestListAttachments_Pagination(t *testing.T) { + srv, slug := testServerWithAttachments(t) + + // Three uploads. realPNG is the only stdlib-decodable shape we + // have; pad each one with a different number of trailing bytes + // after the IEND chunk so the size_bytes column varies. The + // upload handler doesn't re-validate after the IEND — extra + // trailing bytes are stored verbatim, which suits the test fine. + base := realPNG() + mkBody := func(extra int) []byte { + out := make([]byte, len(base)+extra) + copy(out, base) + return out + } + doMultipartUpload(srv, slug, "small.png", mkBody(0)) + doMultipartUpload(srv, slug, "medium.png", mkBody(100)) + doMultipartUpload(srv, slug, "large.png", mkBody(1000)) + + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/attachments", nil) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", rr.Code, rr.Body.String()) + } + var resp struct { + Attachments []struct { + Filename string `json:"filename"` + SizeBytes int64 `json:"size_bytes"` + } `json:"attachments"` + Total int `json:"total"` + Limit int `json:"limit"` + Offset int `json:"offset"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v body=%s", err, rr.Body.String()) + } + if resp.Total != 3 { + t.Errorf("total = %d, want 3", resp.Total) + } + if len(resp.Attachments) != 3 { + t.Fatalf("got %d rows, want 3", len(resp.Attachments)) + } + // Default created_at DESC. Three uploads in the same millisecond + // can land with the same timestamp, so we don't pin the order — + // just assert all three filenames are present in the page. The + // sort=size assertion below covers the ordered case. + gotFilenames := map[string]bool{} + for _, a := range resp.Attachments { + gotFilenames[a.Filename] = true + } + for _, want := range []string{"small.png", "medium.png", "large.png"} { + if !gotFilenames[want] { + t.Errorf("default sort: missing filename %q in result", want) + } + } + + // Ascending size sort. + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/attachments?sort=size", nil) + if rr.Code != http.StatusOK { + t.Fatalf("sort=size status = %d", rr.Code) + } + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Attachments[0].Filename != "small.png" || resp.Attachments[2].Filename != "large.png" { + t.Errorf("sort=size order: %q,%q,%q want small,medium,large", + resp.Attachments[0].Filename, resp.Attachments[1].Filename, resp.Attachments[2].Filename) + } + + // Pagination: limit=2, offset=2 → only 1 row. + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/attachments?limit=2&offset=2", nil) + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Total != 3 { + t.Errorf("total stays 3 across pages, got %d", resp.Total) + } + if len(resp.Attachments) != 1 { + t.Errorf("limit=2 offset=2: got %d rows, want 1", len(resp.Attachments)) + } + if resp.Limit != 2 || resp.Offset != 2 { + t.Errorf("echoed limit/offset = %d/%d, want 2/2", resp.Limit, resp.Offset) + } +} + +// TestListAttachments_HidesDerived asserts that thumbnail rows +// (parent_id != NULL) don't show up in the list. They count toward +// quota but are managed automatically — surfacing them clutters the +// settings page with rows the user didn't upload. +// +// We synthesize a thumbnail row directly via the store rather than +// running the real thumbnail pipeline, which would require a decodable +// image the pure-Go processor accepts. +func TestListAttachments_HidesDerived(t *testing.T) { + srv, slug := testServerWithAttachments(t) + wsID := workspaceIDForSlug(t, srv, slug) + + if rr := doMultipartUpload(srv, slug, "original.png", realPNG()); rr.Code != http.StatusCreated { + t.Fatalf("upload: %d", rr.Code) + } + + // Insert a synthetic thumbnail row pointing at the original. + // The handler should skip it because parent_id IS NOT NULL. + originalID := getOnlyAttachmentID(t, srv, wsID) + thumbVariant := "thumb-sm" + if err := srv.store.CreateAttachment(&models.Attachment{ + WorkspaceID: wsID, + UploadedBy: "system", + StorageKey: "fs:fakehash", + ContentHash: "fakehash", + MimeType: "image/png", + SizeBytes: 123, + Filename: "original-thumb-sm.png", + ParentID: &originalID, + Variant: &thumbVariant, + }); err != nil { + t.Fatalf("CreateAttachment(thumb): %v", err) + } + + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/attachments", nil) + var resp struct { + Attachments []struct{ ID string } `json:"attachments"` + Total int `json:"total"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Total != 1 { + t.Errorf("total = %d, want 1 (derived thumbnail must be hidden)", resp.Total) + } +} + +// TestDeleteAttachment_HappyPath covers the soft-delete flow: a +// successful upload, a 204 from the delete handler, and a follow-up +// list call that no longer sees the row. Storage usage drops to 0 +// confirming the cache invalidation hook fires. +func TestDeleteAttachment_HappyPath(t *testing.T) { + srv, slug := testServerWithAttachments(t) + body := realPNG() + + rr := doMultipartUpload(srv, slug, "victim.png", body) + if rr.Code != http.StatusCreated { + t.Fatalf("upload: %d %s", rr.Code, rr.Body.String()) + } + var upload struct{ ID string } + if err := json.Unmarshal(rr.Body.Bytes(), &upload); err != nil { + t.Fatalf("decode upload: %v", err) + } + + rr = doRequest(srv, "DELETE", "/api/v1/workspaces/"+slug+"/attachments/"+upload.ID, nil) + if rr.Code != http.StatusNoContent { + t.Fatalf("delete: status=%d body=%s", rr.Code, rr.Body.String()) + } + + // List should be empty now. + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/attachments", nil) + var listResp struct { + Total int `json:"total"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &listResp); err != nil { + t.Fatalf("decode list: %v", err) + } + if listResp.Total != 0 { + t.Errorf("after delete: total = %d, want 0", listResp.Total) + } + + // Storage usage drops to 0 (cache was invalidated). + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/storage/usage", nil) + var usage storageUsageResponse + if err := json.Unmarshal(rr.Body.Bytes(), &usage); err != nil { + t.Fatalf("decode usage: %v", err) + } + if usage.UsedBytes != 0 { + t.Errorf("after delete: used_bytes = %d, want 0", usage.UsedBytes) + } + + // Second delete → 404 (already tombstoned). + rr = doRequest(srv, "DELETE", "/api/v1/workspaces/"+slug+"/attachments/"+upload.ID, nil) + if rr.Code != http.StatusNotFound { + t.Errorf("second delete: status=%d, want 404", rr.Code) + } +} + +// TestDeleteAttachment_AfterParentSoftDeleted pins Codex P2 from +// PR #303 round 3: when the parent item is soft-deleted, the +// attachment row remains in the storage list (so the user can see +// it's still consuming quota), and the Delete button must work. +// +// The earlier draft used GetItem which filters out soft-deleted +// items, so the handler returned 404 before ever calling +// SoftDeleteAttachment. This test creates an item, attaches a row, +// soft-deletes the item, and then exercises the delete endpoint. +func TestDeleteAttachment_AfterParentSoftDeleted(t *testing.T) { + srv, slug := testServerWithAttachments(t) + wsID := workspaceIDForSlug(t, srv, slug) + + // Create a real item so the attachment has a parent. Use the + // docs collection (preseeded by the workspace template). + rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/docs/items", + map[string]any{"title": "Doomed", "content": "x"}) + if rr.Code != http.StatusCreated { + t.Fatalf("create item: %d %s", rr.Code, rr.Body.String()) + } + var item struct{ ID string } + if err := json.Unmarshal(rr.Body.Bytes(), &item); err != nil { + t.Fatalf("decode item: %v", err) + } + + // Attach a row to the item directly via the store — easier than + // orchestrating an upload + association sequence. + att := &models.Attachment{ + WorkspaceID: wsID, + ItemID: &item.ID, + UploadedBy: "system", + StorageKey: "fs:" + "x", + ContentHash: "fakehash3", + MimeType: "image/png", + SizeBytes: 123, + Filename: "doomed.png", + } + if err := srv.store.CreateAttachment(att); err != nil { + t.Fatalf("CreateAttachment: %v", err) + } + + // Soft-delete the parent item. + rr = doRequest(srv, "DELETE", "/api/v1/workspaces/"+slug+"/items/"+item.ID, nil) + if rr.Code != http.StatusNoContent && rr.Code != http.StatusOK { + t.Fatalf("soft-delete item: %d %s", rr.Code, rr.Body.String()) + } + + // Attachment should still be visible in the storage list. + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/attachments", nil) + var resp struct { + Total int `json:"total"` + Attachments []store.AttachmentListItem `json:"attachments"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode list: %v", err) + } + if resp.Total != 1 { + t.Errorf("after soft-delete: total=%d, want 1 (attachment must remain visible)", resp.Total) + } + + // Delete must succeed despite the parent item being soft-deleted. + rr = doRequest(srv, "DELETE", "/api/v1/workspaces/"+slug+"/attachments/"+att.ID, nil) + if rr.Code != http.StatusNoContent { + t.Fatalf("delete after parent soft-delete: status=%d body=%s", rr.Code, rr.Body.String()) + } +} + +// TestDeleteAttachment_DerivedRefused pins the carve-out for thumbnail +// rows: a direct delete of a derived attachment should return 400 +// rather than silently succeed (which would leave the original +// without thumbnails until a regenerate job runs). +func TestDeleteAttachment_DerivedRefused(t *testing.T) { + srv, slug := testServerWithAttachments(t) + wsID := workspaceIDForSlug(t, srv, slug) + + if rr := doMultipartUpload(srv, slug, "x.png", realPNG()); rr.Code != http.StatusCreated { + t.Fatalf("upload: %d", rr.Code) + } + originalID := getOnlyAttachmentID(t, srv, wsID) + + thumbVariant := "thumb-sm" + thumb := &models.Attachment{ + WorkspaceID: wsID, + UploadedBy: "system", + StorageKey: "fs:fakehash2", + ContentHash: "fakehash2", + MimeType: "image/png", + SizeBytes: 1, + Filename: "x-thumb-sm.png", + ParentID: &originalID, + Variant: &thumbVariant, + } + if err := srv.store.CreateAttachment(thumb); err != nil { + t.Fatalf("CreateAttachment(thumb): %v", err) + } + + rr := doRequest(srv, "DELETE", "/api/v1/workspaces/"+slug+"/attachments/"+thumb.ID, nil) + if rr.Code != http.StatusBadRequest { + t.Errorf("delete derived: status=%d, want 400", rr.Code) + } +} + +// getOnlyAttachmentID returns the ID of the single live attachment +// row in a workspace — the test infrastructure uploads at most one +// blob per call, so this lookup is deterministic. Fails the test if +// zero or multiple rows are live. +func getOnlyAttachmentID(t *testing.T, srv *Server, workspaceID string) string { + t.Helper() + rows, _, err := srv.store.WorkspaceAttachments(workspaceID, store.AttachmentListFilters{}) + if err != nil { + t.Fatalf("WorkspaceAttachments: %v", err) + } + if len(rows) != 1 { + t.Fatalf("expected exactly 1 attachment, got %d", len(rows)) + } + return rows[0].ID +} + +// workspaceIDForSlug looks up the internal UUID for a slug — used +// by tests that synthesize attachment rows directly via the store +// (bypassing the upload handler). +func workspaceIDForSlug(t *testing.T, srv *Server, slug string) string { + t.Helper() + ws, err := srv.store.GetWorkspaceBySlug(slug) + if err != nil { + t.Fatalf("GetWorkspaceBySlug(%q): %v", slug, err) + } + if ws == nil { + t.Fatalf("workspace %q not found", slug) + } + return ws.ID +} + // TestStorageInfoCache covers the in-memory cache directly so the TTL + // invalidate paths have a focused test. The handler-level integration // is exercised by TestStorageUsage_TracksUploads (which depends on diff --git a/internal/server/server.go b/internal/server/server.go index 4a8ca53d..a85c797a 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -731,9 +731,11 @@ func (s *Server) setupRouter() { // path on HEAD; http.ServeContent already strips the body // on the seekable path. r.Post("/attachments", s.handleUploadAttachment) + r.Get("/attachments", s.handleListWorkspaceAttachments) r.Get("/attachments/{attachmentID}", s.handleGetAttachment) r.Head("/attachments/{attachmentID}", s.handleGetAttachment) r.Post("/attachments/{attachmentID}/transform", s.handleTransformAttachment) + r.Delete("/attachments/{attachmentID}", s.handleDeleteWorkspaceAttachment) // Storage usage summary for Settings → Storage and other // quota-aware UI surfaces (TASK-881). Cached behind a diff --git a/internal/store/attachments.go b/internal/store/attachments.go index ddef1d80..1036e8b8 100644 --- a/internal/store/attachments.go +++ b/internal/store/attachments.go @@ -4,6 +4,7 @@ import ( "database/sql" "encoding/json" "fmt" + "strings" "github.com/PerpetualSoftware/pad/internal/models" ) @@ -127,6 +128,432 @@ func (s *Store) GetAttachmentVariant(parentID, variant string) (*models.Attachme return a, nil } +// AttachmentListFilters narrow the WorkspaceAttachments result set. +// Zero values mean "no filter on this dimension". The handler turns +// query-string parameters into this struct so the SQL builder stays +// pure and unit-testable. +type AttachmentListFilters struct { + // MimeCategory restricts to a single MIME category bucket + // (matches attachments.Category — "image", "document", etc.). + // Empty = all categories. Translated to MIME predicates per + // mimePredicateForCategory; categories without a clean prefix + // (document, text, archive, other) use an explicit IN list. + MimeCategory string + + // Attached restricts to attachments associated with an item. + Attached bool + + // Unattached restricts to orphan attachments (item_id IS NULL). + // Mutually exclusive with Attached — handler validates upstream. + Unattached bool + + // CollectionID restricts to attachments belonging to items in + // this collection. Empty = no collection filter. + CollectionID string + + // Sort field. Accepts "size", "filename", "created_at" with an + // optional " desc" suffix. Empty = "created_at desc" (newest first). + Sort string + + // Limit caps the page size. Clamped to [1, 200] by the handler. + Limit int + + // Offset pages forward. Combined with Limit + Total for the UI's + // classic page navigator. Negative values clamped to 0. + Offset int + + // Restricted, FullCollectionIDs, GrantedItemIDs together encode + // per-user collection + item visibility. When Restricted is true + // the list filters to attachments whose parent item is either + // in one of FullCollectionIDs or has its id in GrantedItemIDs. + // Orphans (item_id IS NULL) are excluded from restricted views + // so a member who only sees one collection can't enumerate + // filenames of unattached uploads from collections they don't + // have access to. + // + // Restricted=false → no filter (admin / full-access member). + // Restricted=true with empty Full+Granted → zero rows. + // Restricted=true with one or both populated → SQL OR of the + // two predicates. + // + // Mirrors the (fullCollIDs, grantedItemIDs) tuple returned by + // Server.guestResourceFilter — keep the semantics in sync. + Restricted bool + FullCollectionIDs []string + GrantedItemIDs []string +} + +// AttachmentListItem is a row from WorkspaceAttachments enriched with +// the parent item's title + slug + collection slug so the UI can render +// a clickable link without a follow-up GET. Item fields are nullable +// for orphan rows. +// +// URL construction: the item route is /{user}/{ws}/{collection_slug}/{item_slug}, +// so the UI uses ItemSlug — never a synthetic "TASK-5"-style ref. The +// ref shape isn't 1:1 with the route, and exposing it here led to a +// double-collection-slug bug in an earlier draft. +// +// ItemDeleted is true when the parent item exists but has been soft- +// deleted. The row is still surfaced because the bytes still consume +// quota; the UI uses the flag to render "(deleted)" instead of a +// clickable link to a 404'd item. +type AttachmentListItem struct { + models.Attachment + ItemTitle *string `json:"item_title,omitempty"` + ItemSlug *string `json:"item_slug,omitempty"` + ItemDeleted bool `json:"item_deleted,omitempty"` + CollectionSlug *string `json:"collection_slug,omitempty"` +} + +// allowedAttachmentSorts pins the columns + directions the list +// endpoint accepts, so a hand-crafted sort= query can't smuggle in +// arbitrary SQL. Map values are the literal SQL fragment we splice in. +var allowedAttachmentSorts = map[string]string{ + "size": "a.size_bytes ASC", + "size_desc": "a.size_bytes DESC", + "filename": "a.filename ASC", + "filename_desc": "a.filename DESC", + "created_at": "a.created_at ASC", + "created_at_desc": "a.created_at DESC", +} + +// mimePredicateForCategory maps an attachments.Category value to a +// SQL fragment + matching argument list that selects all MIMEs in +// that bucket. Categories with a clean type prefix (image/, video/, +// audio/) use a LIKE; the rest use an explicit IN list mirroring +// the entries in internal/attachments/mime.go. +// +// Returns (frag, args, true) when the category is known. The frag +// uses ? placeholders that the caller splices into the WHERE. ok=false +// for unknown categories — caller must skip the filter so the UI +// shows everything rather than zero rows for typos. +// +// Keep the literal MIME lists in lockstep with internal/attachments/ +// mime.go: any time a new MIME is added to the allowlist there, mirror +// it here so the Settings → Storage filter stays useful. +func mimePredicateForCategory(category string) (frag string, args []any, ok bool) { + switch category { + case "image": + return "a.mime_type LIKE ?", []any{"image/%"}, true + case "video": + return "a.mime_type LIKE ?", []any{"video/%"}, true + case "audio": + return "a.mime_type LIKE ?", []any{"audio/%"}, true + case "document": + return mimeInPredicate([]string{ + "application/pdf", + "application/msword", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + "application/vnd.ms-excel", + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + "application/vnd.ms-powerpoint", + "application/vnd.openxmlformats-officedocument.presentationml.presentation", + "application/vnd.oasis.opendocument.text", + "application/vnd.oasis.opendocument.spreadsheet", + "application/vnd.oasis.opendocument.presentation", + "application/rtf", + }) + case "text": + return mimeInPredicate([]string{ + "text/plain", "text/markdown", "text/csv", "text/tab-separated-values", + "application/json", "application/xml", "text/xml", + "application/yaml", "text/yaml", "application/toml", + "text/html", "text/javascript", "application/javascript", + }) + case "archive": + return mimeInPredicate([]string{ + "application/zip", "application/x-tar", "application/gzip", + "application/x-bzip2", "application/x-7z-compressed", + }) + case "other": + // "Other" is the negation of every named bucket. Easier to + // build by exclusion: NOT in the union of all known MIMEs. + // Listing the categories explicitly keeps this in lockstep + // with the named buckets above without a second source of + // truth. + all := []string{ + "application/pdf", "application/msword", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + "application/vnd.ms-excel", + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + "application/vnd.ms-powerpoint", + "application/vnd.openxmlformats-officedocument.presentationml.presentation", + "application/vnd.oasis.opendocument.text", + "application/vnd.oasis.opendocument.spreadsheet", + "application/vnd.oasis.opendocument.presentation", + "application/rtf", + "text/plain", "text/markdown", "text/csv", "text/tab-separated-values", + "application/json", "application/xml", "text/xml", + "application/yaml", "text/yaml", "application/toml", + "text/html", "text/javascript", "application/javascript", + "application/zip", "application/x-tar", "application/gzip", + "application/x-bzip2", "application/x-7z-compressed", + } + placeholders := make([]string, len(all)) + args := make([]any, len(all)) + for i, m := range all { + placeholders[i] = "?" + args[i] = m + } + frag := "a.mime_type NOT LIKE 'image/%' AND a.mime_type NOT LIKE 'video/%' AND a.mime_type NOT LIKE 'audio/%' AND a.mime_type NOT IN (" + strings.Join(placeholders, ",") + ")" + return frag, args, true + } + return "", nil, false +} + +// mimeInPredicate builds a `a.mime_type IN (?,?,...)` fragment with +// matching args. Helper used by mimePredicateForCategory for the +// non-prefix categories. +func mimeInPredicate(mimes []string) (string, []any, bool) { + placeholders := make([]string, len(mimes)) + args := make([]any, len(mimes)) + for i, m := range mimes { + placeholders[i] = "?" + args[i] = m + } + return "a.mime_type IN (" + strings.Join(placeholders, ",") + ")", args, true +} + +// WorkspaceAttachments lists original (non-derived) attachments in a +// workspace, with optional filtering + sorting + pagination. Returns +// the page rows and the total count of matching rows so the UI can +// render a paginator without a second round-trip. +// +// Intentionally hides derived blobs (thumbnails — rows where +// parent_id IS NOT NULL): they're managed automatically and showing +// them in the list would clutter the page with rows the user didn't +// upload. They still count against storage quota via +// WorkspaceStorageUsage; the totals match the bar even when the list +// shows only originals. +// +// LEFT JOIN to items + collections gives the UI everything it needs +// to render an "in [[Task X]]" link. Soft-deleted items are returned +// with item fields nulled out — the attachment is still visible +// (a deleted item could still be restored), but the link target +// isn't reachable. +func (s *Store) WorkspaceAttachments(workspaceID string, filters AttachmentListFilters) ([]AttachmentListItem, int, error) { + // Build the WHERE clause incrementally. Every branch parameter + // goes through the placeholder slice — no string concatenation of + // user input. Sort is the only user-controllable splice and it + // goes through the allowedAttachmentSorts allowlist. + var conds []string + var args []any + + conds = append(conds, "a.workspace_id = ?") + args = append(args, workspaceID) + conds = append(conds, "a.deleted_at IS NULL") + conds = append(conds, "a.parent_id IS NULL") // hide derived blobs + + if filters.Attached { + conds = append(conds, "a.item_id IS NOT NULL") + } + if filters.Unattached { + conds = append(conds, "a.item_id IS NULL") + } + if frag, mimeArgs, ok := mimePredicateForCategory(filters.MimeCategory); ok { + conds = append(conds, frag) + args = append(args, mimeArgs...) + } + if filters.CollectionID != "" { + conds = append(conds, "i.collection_id = ?") + args = append(args, filters.CollectionID) + } + + // Collection + item-level visibility enforcement. Two sources of + // access: collections the user can see in full (FullCollectionIDs) + // and individual items granted to them (GrantedItemIDs). The + // predicate ORs them so an item-grant in a hidden collection still + // resolves; orphans (item_id IS NULL) are excluded entirely so a + // restricted user can't enumerate orphan filenames. + // + // Restricted=false → no filter (admin / full-access member). + // Restricted=true with both lists empty → zero rows. + if filters.Restricted { + var ors []string + if len(filters.FullCollectionIDs) > 0 { + ph := make([]string, len(filters.FullCollectionIDs)) + for i, id := range filters.FullCollectionIDs { + ph[i] = "?" + args = append(args, id) + } + ors = append(ors, "i.collection_id IN ("+strings.Join(ph, ",")+")") + } + if len(filters.GrantedItemIDs) > 0 { + ph := make([]string, len(filters.GrantedItemIDs)) + for i, id := range filters.GrantedItemIDs { + ph[i] = "?" + args = append(args, id) + } + ors = append(ors, "a.item_id IN ("+strings.Join(ph, ",")+")") + } + if len(ors) == 0 { + conds = append(conds, "1 = 0") + } else { + conds = append(conds, "("+strings.Join(ors, " OR ")+")") + } + } + + where := strings.Join(conds, " AND ") + + // Count total before applying limit/offset so the UI can render + // "showing 1–25 of 312". + // + // The items LEFT JOIN intentionally does NOT filter on + // items.deleted_at — attachments survive a soft-deleted parent + // (they still consume quota), so the storage list must surface + // them and the collection-level visibility predicate + // (i.collection_id IN ...) must keep working. The handler's + // delete path uses GetItemIncludeDeleted for the same reason. + var total int + if err := s.db.QueryRow(s.q(` + SELECT COUNT(*) FROM attachments a + LEFT JOIN items i ON i.id = a.item_id + WHERE `+where), args...).Scan(&total); err != nil { + return nil, 0, fmt.Errorf("count workspace attachments: %w", err) + } + + orderBy, ok := allowedAttachmentSorts[filters.Sort] + if !ok { + orderBy = "a.created_at DESC" // sensible default — newest first + } + + limit := filters.Limit + if limit <= 0 { + limit = 50 + } + if limit > 200 { + limit = 200 + } + offset := filters.Offset + if offset < 0 { + offset = 0 + } + + // Column list is the same as attachmentColumns but prefixed with + // `a.` so SQLite doesn't choke on the ambiguous `id` shared with + // the joined items table. + const aliasedAttachmentColumns = `a.id, a.workspace_id, a.item_id, a.uploaded_by, a.storage_key, a.content_hash, + a.mime_type, a.size_bytes, a.filename, a.width, a.height, a.parent_id, a.variant, a.created_at, a.deleted_at` + + // Same rationale as the count query above: include soft-deleted + // parent items so the row is still visible for users who would + // be allowed to see the live item, and so the collection-level + // ACL predicate sees a non-NULL i.collection_id. The response + // surfaces deleted_at on the joined item via item_deleted so the + // UI can render a "(deleted)" tag instead of a clickable link. + q := ` + SELECT ` + aliasedAttachmentColumns + `, + i.title, i.slug, i.deleted_at, + c.slug, c.name + FROM attachments a + LEFT JOIN items i ON i.id = a.item_id + LEFT JOIN collections c ON c.id = i.collection_id + WHERE ` + where + ` + ORDER BY ` + orderBy + ` + LIMIT ? OFFSET ?` + + args = append(args, limit, offset) + rows, err := s.db.Query(s.q(q), args...) + if err != nil { + return nil, 0, fmt.Errorf("list workspace attachments: %w", err) + } + defer rows.Close() + + var out []AttachmentListItem + for rows.Next() { + var a models.Attachment + var itemID, parentID, variant, deletedAt *string + var width, height *int + var createdAt string + + // Item + collection columns from the LEFT JOIN. All nullable. + var itemTitle, itemSlug, itemDeletedAt *string + var collSlug, collName *string + + if err := rows.Scan( + &a.ID, &a.WorkspaceID, &itemID, &a.UploadedBy, &a.StorageKey, &a.ContentHash, + &a.MimeType, &a.SizeBytes, &a.Filename, &width, &height, + &parentID, &variant, &createdAt, &deletedAt, + &itemTitle, &itemSlug, &itemDeletedAt, + &collSlug, &collName, + ); err != nil { + return nil, 0, fmt.Errorf("scan workspace attachment: %w", err) + } + a.ItemID = itemID + a.ParentID = parentID + a.Variant = variant + a.Width = width + a.Height = height + a.CreatedAt = parseTime(createdAt) + a.DeletedAt = parseTimePtr(deletedAt) + + row := AttachmentListItem{Attachment: a} + if itemTitle != nil { + row.ItemTitle = itemTitle + } + if itemSlug != nil { + row.ItemSlug = itemSlug + } + if itemDeletedAt != nil && *itemDeletedAt != "" { + row.ItemDeleted = true + } + if collSlug != nil { + row.CollectionSlug = collSlug + } + out = append(out, row) + } + if err := rows.Err(); err != nil { + return nil, 0, fmt.Errorf("iterate workspace attachments: %w", err) + } + return out, total, nil +} + +// SoftDeleteAttachment marks the given attachment row deleted (and +// every variant whose parent_id points at it) so the orphan GC will +// reclaim the bytes after the grace period. Returns sql.ErrNoRows if +// no live row matches. +// +// We mark variants via a separate UPDATE keyed on parent_id rather +// than letting a foreign-key cascade do it — the attachments table +// has no FK on parent_id, by design (DOC-865: thumbnails are +// independent rows so a missing original doesn't break a list query). +// +// The blob on disk is NOT removed here; the same content_hash may be +// referenced by other rows (content-addressed dedupe), so reclamation +// is the GC's job once it can prove no live row references the hash. +func (s *Store) SoftDeleteAttachment(id string) error { + ts := now() + res, err := s.db.Exec(s.q(` + UPDATE attachments + SET deleted_at = ? + WHERE id = ? AND deleted_at IS NULL + `), ts, id) + if err != nil { + return fmt.Errorf("soft delete attachment: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("soft delete attachment rows affected: %w", err) + } + if n == 0 { + return sql.ErrNoRows + } + // Also tombstone any thumbnail variants. They're synthetic rows + // derived from the original; without the original they have no + // reason to exist. Errors here are non-fatal — orphan GC will + // still reach them eventually via the deleted-parent path. + if _, err := s.db.Exec(s.q(` + UPDATE attachments + SET deleted_at = ? + WHERE parent_id = ? AND deleted_at IS NULL + `), ts, id); err != nil { + // Log via the caller; we don't have a logger here. The row + // went through, so don't fail the request. + return nil + } + return nil +} + // WorkspaceStorageUsage returns the total bytes consumed by non-deleted // attachments in the workspace. Includes derived blobs (thumbnails) — // those are real bytes on disk and count against quota. diff --git a/internal/store/attachments_test.go b/internal/store/attachments_test.go index 7894cb0b..37d2d84f 100644 --- a/internal/store/attachments_test.go +++ b/internal/store/attachments_test.go @@ -121,6 +121,286 @@ func TestWorkspaceStorageInfo_FreePlanResolution(t *testing.T) { } } +// TestWorkspaceAttachments_VisibilityFilter verifies that +// VisibleCollectionIDs gates the result set per Codex P1 from +// PR #303 round 1: a restricted member must not see attachments +// in collections they can't access, and orphans (item_id IS NULL) +// must be hidden as well so filenames don't leak. +func TestWorkspaceAttachments_VisibilityFilter(t *testing.T) { + s := testStore(t) + + wsID := newID() + collA := newID() + collB := newID() + itemA := newID() + itemB := newID() + ts := time.Now().UTC().Format(time.RFC3339) + if _, err := s.db.Exec(s.q(`INSERT INTO workspaces (id, slug, name, settings, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?)`), + wsID, "ws", "WS", "{}", ts, ts); err != nil { + t.Fatalf("insert workspace: %v", err) + } + for _, c := range []struct{ id, slug, name string }{{collA, "tasks", "Tasks"}, {collB, "secrets", "Secrets"}} { + if _, err := s.db.Exec(s.q(`INSERT INTO collections (id, workspace_id, name, slug, schema, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?)`), + c.id, wsID, c.name, c.slug, `{"fields":[]}`, ts, ts); err != nil { + t.Fatalf("insert collection %s: %v", c.slug, err) + } + } + mkItem := func(id, collID, slug, title string) { + t.Helper() + if _, err := s.db.Exec(s.q(`INSERT INTO items (id, workspace_id, collection_id, title, slug, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?)`), + id, wsID, collID, title, slug, ts, ts); err != nil { + t.Fatalf("insert item: %v", err) + } + } + mkItem(itemA, collA, "task-1", "Task 1") + mkItem(itemB, collB, "secret-1", "Secret") + + mkAttach := func(itemID *string, filename string) { + t.Helper() + a := &models.Attachment{ + WorkspaceID: wsID, + ItemID: itemID, + UploadedBy: "system", + StorageKey: "fs:" + newID(), + ContentHash: newID(), + MimeType: "image/png", + SizeBytes: 100, + Filename: filename, + } + if err := s.CreateAttachment(a); err != nil { + t.Fatalf("CreateAttachment: %v", err) + } + } + mkAttach(&itemA, "task-screenshot.png") + mkAttach(&itemB, "secret-screenshot.png") + mkAttach(nil, "orphan.png") + + // Admin / unrestricted (nil) sees everything. + rows, total, err := s.WorkspaceAttachments(wsID, AttachmentListFilters{}) + if err != nil { + t.Fatalf("admin list: %v", err) + } + if total != 3 || len(rows) != 3 { + t.Errorf("admin: total=%d rows=%d, want 3/3", total, len(rows)) + } + + // Restricted to tasks only: see task-screenshot, hide secret + orphan. + rows, total, err = s.WorkspaceAttachments(wsID, AttachmentListFilters{ + Restricted: true, + FullCollectionIDs: []string{collA}, + }) + if err != nil { + t.Fatalf("restricted list: %v", err) + } + if total != 1 || len(rows) != 1 { + t.Fatalf("restricted: total=%d rows=%d, want 1/1", total, len(rows)) + } + if rows[0].Filename != "task-screenshot.png" { + t.Errorf("restricted: filename=%q, want task-screenshot.png", rows[0].Filename) + } + + // Item-level grant only: a restricted user with a single granted + // item in collB should see only that item's attachment, not the + // rest of collB's contents. Mirrors handlers_search's + // (fullCollIDs, grantedItemIDs) tuple. + rows, total, err = s.WorkspaceAttachments(wsID, AttachmentListFilters{ + Restricted: true, + GrantedItemIDs: []string{itemB}, + }) + if err != nil { + t.Fatalf("item-grant list: %v", err) + } + if total != 1 || len(rows) != 1 { + t.Fatalf("item-grant: total=%d rows=%d, want 1/1", total, len(rows)) + } + if rows[0].Filename != "secret-screenshot.png" { + t.Errorf("item-grant: filename=%q, want secret-screenshot.png", rows[0].Filename) + } + + // Empty visibility (restricted with no collections + no item + // grants) — zero rows. + rows, total, err = s.WorkspaceAttachments(wsID, AttachmentListFilters{ + Restricted: true, + }) + if err != nil { + t.Fatalf("zero-visibility list: %v", err) + } + if total != 0 || len(rows) != 0 { + t.Errorf("zero-visibility: total=%d rows=%d, want 0/0", total, len(rows)) + } +} + +// TestWorkspaceAttachments_SurfacesSoftDeletedParents pins Codex P2 +// from PR #303 round 4: an attachment whose parent item is soft- +// deleted must remain in the list so the user can reclaim the +// bytes, AND the collection-level visibility filter must still see +// the (still-set) collection_id so restricted users with access to +// that collection can find the attachment. +// +// Two assertions: +// - Full-access caller (Restricted=false) sees the row. +// - Restricted caller scoped to the right collection sees the row; +// restricted to a different collection does not. +// - The row carries item_deleted=true so the UI can render the +// "(deleted)" badge. +func TestWorkspaceAttachments_SurfacesSoftDeletedParents(t *testing.T) { + s := testStore(t) + + wsID := newID() + collA := newID() + collB := newID() + itemA := newID() + ts := time.Now().UTC().Format(time.RFC3339) + if _, err := s.db.Exec(s.q(`INSERT INTO workspaces (id, slug, name, settings, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?)`), + wsID, "ws", "WS", "{}", ts, ts); err != nil { + t.Fatalf("insert workspace: %v", err) + } + for _, c := range []struct{ id, slug, name string }{{collA, "tasks", "Tasks"}, {collB, "ideas", "Ideas"}} { + if _, err := s.db.Exec(s.q(`INSERT INTO collections (id, workspace_id, name, slug, schema, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?)`), + c.id, wsID, c.name, c.slug, `{"fields":[]}`, ts, ts); err != nil { + t.Fatalf("insert collection: %v", err) + } + } + // Soft-deleted item — deleted_at is set. + if _, err := s.db.Exec(s.q(`INSERT INTO items (id, workspace_id, collection_id, title, slug, created_at, updated_at, deleted_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`), + itemA, wsID, collA, "Doomed", "doomed", ts, ts, ts); err != nil { + t.Fatalf("insert deleted item: %v", err) + } + if err := s.CreateAttachment(&models.Attachment{ + WorkspaceID: wsID, + ItemID: &itemA, + UploadedBy: "system", + StorageKey: "fs:" + newID(), + ContentHash: newID(), + MimeType: "image/png", + SizeBytes: 100, + Filename: "doomed.png", + }); err != nil { + t.Fatalf("CreateAttachment: %v", err) + } + + // 1. Admin / full-access sees the row + item_deleted flag. + rows, total, err := s.WorkspaceAttachments(wsID, AttachmentListFilters{}) + if err != nil { + t.Fatalf("admin list: %v", err) + } + if total != 1 || len(rows) != 1 { + t.Fatalf("admin: total=%d rows=%d, want 1/1", total, len(rows)) + } + if !rows[0].ItemDeleted { + t.Errorf("admin: ItemDeleted=false, want true (parent is soft-deleted)") + } + if rows[0].ItemTitle == nil || *rows[0].ItemTitle != "Doomed" { + t.Errorf("admin: item_title=%v, want Doomed (soft-deleted parent's title still surfaces)", rows[0].ItemTitle) + } + + // 2. Restricted to collA sees the row. + rows, total, err = s.WorkspaceAttachments(wsID, AttachmentListFilters{ + Restricted: true, + FullCollectionIDs: []string{collA}, + }) + if err != nil { + t.Fatalf("restricted-collA list: %v", err) + } + if total != 1 || len(rows) != 1 { + t.Fatalf("restricted-collA: total=%d rows=%d, want 1/1", total, len(rows)) + } + + // 3. Restricted to collB does NOT see the row. + rows, total, err = s.WorkspaceAttachments(wsID, AttachmentListFilters{ + Restricted: true, + FullCollectionIDs: []string{collB}, + }) + if err != nil { + t.Fatalf("restricted-collB list: %v", err) + } + if total != 0 || len(rows) != 0 { + t.Errorf("restricted-collB: total=%d rows=%d, want 0/0", total, len(rows)) + } +} + +// TestWorkspaceAttachments_CategoryFilters covers the document/text/ +// archive/other filter buckets per Codex P2 from PR #303 round 1: +// the earlier prefix-only mapping silently passed those filters +// through with no MIME predicate, returning the full list. +func TestWorkspaceAttachments_CategoryFilters(t *testing.T) { + s := testStore(t) + + wsID := newID() + ts := time.Now().UTC().Format(time.RFC3339) + if _, err := s.db.Exec(s.q(`INSERT INTO workspaces (id, slug, name, settings, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?)`), + wsID, "ws", "WS", "{}", ts, ts); err != nil { + t.Fatalf("insert workspace: %v", err) + } + + mk := func(mime, filename string) { + t.Helper() + a := &models.Attachment{ + WorkspaceID: wsID, + UploadedBy: "system", + StorageKey: "fs:" + newID(), + ContentHash: newID(), + MimeType: mime, + SizeBytes: 1, + Filename: filename, + } + if err := s.CreateAttachment(a); err != nil { + t.Fatalf("CreateAttachment: %v", err) + } + } + mk("image/png", "a.png") + mk("application/pdf", "b.pdf") + mk("text/markdown", "c.md") + mk("application/zip", "d.zip") + mk("application/octet-stream", "e.bin") // not in any named bucket → "other" + + cases := []struct { + category string + want []string + }{ + {"image", []string{"a.png"}}, + {"document", []string{"b.pdf"}}, + {"text", []string{"c.md"}}, + {"archive", []string{"d.zip"}}, + {"other", []string{"e.bin"}}, + } + for _, tc := range cases { + t.Run(tc.category, func(t *testing.T) { + rows, total, err := s.WorkspaceAttachments(wsID, AttachmentListFilters{ + MimeCategory: tc.category, + }) + if err != nil { + t.Fatalf("list: %v", err) + } + if total != len(tc.want) { + t.Fatalf("total=%d, want %d", total, len(tc.want)) + } + got := make([]string, len(rows)) + for i, r := range rows { + got[i] = r.Filename + } + for _, want := range tc.want { + found := false + for _, g := range got { + if g == want { + found = true + } + } + if !found { + t.Errorf("missing %q in result %v", want, got) + } + } + }) + } +} + // TestWorkspaceStorageInfo_TracksLiveAttachments inserts a few // attachment rows directly and asserts SUM(size_bytes) shows up in // used_bytes — and that soft-deleted rows are excluded so the user diff --git a/internal/store/items.go b/internal/store/items.go index f93c6968..c6b029af 100644 --- a/internal/store/items.go +++ b/internal/store/items.go @@ -392,6 +392,58 @@ func parseItemRef(s string) (string, int, bool) { return prefix, num, true } +// GetItemIncludeDeleted finds an item by id including soft-deleted +// items. Used by code paths that need to act on records the user +// already owns even though the parent item has been moved to trash — +// the most common case is the Settings → Storage attachment list, +// where attachments survive a soft-deleted parent (so the user can +// see what's still consuming quota and decide whether to delete the +// blob). The visibility check still keys off the (still-set) +// collection_id, so soft-deleting an item doesn't escalate access. +func (s *Store) GetItemIncludeDeleted(id string) (*models.Item, error) { + var item models.Item + var createdAt, updatedAt string + var deletedAt *string + var pinned bool + + err := s.db.QueryRow(s.q(` + SELECT i.id, i.workspace_id, i.collection_id, i.title, i.slug, i.content, i.fields, i.tags, + i.pinned, i.sort_order, i.parent_id, i.assigned_user_id, i.agent_role_id, i.role_sort_order, + i.created_by, i.last_modified_by, i.source, + i.item_number, i.created_at, i.updated_at, i.deleted_at, + c.slug, c.name, c.icon, c.prefix, + COALESCE(au.name, ''), COALESCE(au.email, ''), + COALESCE(ar.name, ''), COALESCE(ar.slug, ''), COALESCE(ar.icon, '') + FROM items i + JOIN collections c ON c.id = i.collection_id + LEFT JOIN users au ON au.id = i.assigned_user_id + LEFT JOIN agent_roles ar ON ar.id = i.agent_role_id + WHERE i.id = ? + `), id).Scan( + &item.ID, &item.WorkspaceID, &item.CollectionID, &item.Title, &item.Slug, + &item.Content, &item.Fields, &item.Tags, + &pinned, &item.SortOrder, &item.ParentID, &item.AssignedUserID, &item.AgentRoleID, &item.RoleSortOrder, + &item.CreatedBy, &item.LastModifiedBy, &item.Source, + &item.ItemNumber, &createdAt, &updatedAt, &deletedAt, + &item.CollectionSlug, &item.CollectionName, &item.CollectionIcon, &item.CollectionPrefix, + &item.AssignedUserName, &item.AssignedUserEmail, + &item.AgentRoleName, &item.AgentRoleSlug, &item.AgentRoleIcon, + ) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("get item (include deleted): %w", err) + } + + item.Pinned = pinned + item.CreatedAt = parseTime(createdAt) + item.UpdatedAt = parseTime(updatedAt) + item.DeletedAt = parseTimePtr(deletedAt) + hydrateItemComputedMetadata(&item) + return &item, nil +} + // GetItemBySlugIncludeDeleted finds an item by slug including soft-deleted items. // Used for restore operations where the item is archived. func (s *Store) GetItemBySlugIncludeDeleted(workspaceID, slug string) (*models.Item, error) { diff --git a/web/src/lib/api/client.ts b/web/src/lib/api/client.ts index b552a8a0..8aa3a26b 100644 --- a/web/src/lib/api/client.ts +++ b/web/src/lib/api/client.ts @@ -46,7 +46,9 @@ import type { AttachmentTransformRequest, AttachmentTransformResult, ServerCapabilities, - WorkspaceStorageInfo + WorkspaceStorageInfo, + AttachmentListFilters, + AttachmentListResponse } from '$lib/types'; const BASE = '/api/v1'; @@ -860,6 +862,49 @@ export const api = { return request( `/workspaces/${workspaceSlug}/storage/usage` ); + }, + + /** + * Paginated list of attachments in a workspace, used by the + * Settings → Storage page. Hides derived blobs (thumbnails) by + * default — those are managed automatically and shouldn't show + * as user-visible rows. + * + * `total` in the response is the count of all matching rows + * (across all pages); pair it with `limit` + `offset` to render + * a classic paginator. Server clamps limit to [1, 200]. + */ + list( + workspaceSlug: string, + filters: AttachmentListFilters = {} + ): Promise { + const params = new URLSearchParams(); + if (filters.category) params.set('category', filters.category); + if (filters.item) params.set('item', filters.item); + if (filters.collection) params.set('collection', filters.collection); + if (filters.sort) params.set('sort', filters.sort); + if (filters.limit !== undefined) params.set('limit', String(filters.limit)); + if (filters.offset !== undefined) params.set('offset', String(filters.offset)); + const qs = params.toString(); + const suffix = qs ? `?${qs}` : ''; + return request( + `/workspaces/${workspaceSlug}/attachments${suffix}` + ); + }, + + /** + * Soft-delete an attachment by ID. The blob on disk stays put + * (content-addressed dedupe means the same hash may still be + * referenced) — orphan GC reclaims past the grace period. + * + * Returns 204 No Content. Refuses to delete derived + * (thumbnail) rows — caller must delete the original. + */ + async delete(workspaceSlug: string, attachmentId: string): Promise { + await request( + `/workspaces/${workspaceSlug}/attachments/${attachmentId}`, + { method: 'DELETE' } + ); } }, diff --git a/web/src/lib/components/settings/StorageTab.svelte b/web/src/lib/components/settings/StorageTab.svelte new file mode 100644 index 00000000..a4e5c8e0 --- /dev/null +++ b/web/src/lib/components/settings/StorageTab.svelte @@ -0,0 +1,701 @@ + + +
+ {#if loading} +

Loading storage…

+ {:else} + +
+ {#if usage} +
+ {#if usage.limit_bytes >= 0} + + {formatBytes(usage.used_bytes)} + used of + {formatBytes(usage.limit_bytes)} + ({usagePercent.toFixed(1)}%) + + {:else} + + {formatBytes(usage.used_bytes)} + used + (unlimited) + + {/if} + {#if usage.override_active} + custom override + {/if} +
+ + {#if usage.limit_bytes >= 0} +
+
+
+ {/if} + + {#if usage.override_active && usage.plan} +
+ Plan: {usage.plan} — admin override active +
+ {/if} + {:else} +

Unable to load usage info.

+ {/if} +
+ + +
+ + + + + +
+ + + {#if total === 0} +

+ No attachments yet — paste or drag a file into any item to upload one. +

+ {:else} +
+ {#each attachments as att (att.id)} +
+ + {#if isImage(att.mime_type)} + {att.filename} + {:else} + + {/if} + + +
+
+ {att.filename} +
+
+ {formatBytes(att.size_bytes)} + · + {att.mime_type} + · + {formatDate(att.created_at)} +
+
+ {#if att.item_title && att.collection_slug} + in + {#if att.item_deleted} + + [[{att.item_title}]] + deleted + {:else} + [[{att.item_title}]] + {/if} + {:else} + Unattached + {/if} +
+
+ +
+ +
+
+ {/each} +
+ + +
+ Showing {pageStart}–{pageEnd} of {total} +
+ + +
+
+ {/if} + {/if} +
+ + diff --git a/web/src/lib/types/index.ts b/web/src/lib/types/index.ts index 33f37f51..191a407f 100644 --- a/web/src/lib/types/index.ts +++ b/web/src/lib/types/index.ts @@ -840,6 +840,68 @@ export interface WorkspaceStorageInfo { override_active: boolean; } +/** + * Row shape from GET /api/v1/workspaces/{ws}/attachments. Mirrors + * the store's AttachmentListItem — base attachment columns plus + * LEFT JOIN'd item title / slug / collection slug for the "in + * [[Item X]]" link in the settings page. Item fields are absent + * for orphan attachments. + */ +export interface AttachmentListItem { + id: string; + workspace_id: string; + item_id?: string | null; + uploaded_by: string; + storage_key: string; + content_hash: string; + mime_type: string; + size_bytes: number; + filename: string; + width?: number | null; + height?: number | null; + parent_id?: string | null; + variant?: string | null; + created_at: string; + deleted_at?: string | null; + item_title?: string | null; + item_slug?: string | null; + /** + * True when the parent item is soft-deleted. The attachment is + * still surfaced (the bytes still count toward quota) but the + * UI should render "(deleted)" instead of a clickable link. + */ + item_deleted?: boolean; + collection_slug?: string | null; +} + +/** + * Paginated response from GET /api/v1/workspaces/{ws}/attachments. + * `total` is the count of all matching rows (across all pages); the + * UI uses it with `limit` + `offset` to render a classic paginator. + */ +export interface AttachmentListResponse { + attachments: AttachmentListItem[]; + total: number; + limit: number; + offset: number; +} + +/** Filters accepted by attachments.list — translated to query params. */ +export interface AttachmentListFilters { + category?: 'image' | 'video' | 'audio' | 'document' | 'text' | 'archive' | 'other'; + item?: 'attached' | 'unattached'; + collection?: string; + sort?: + | 'size' + | 'size_desc' + | 'filename' + | 'filename_desc' + | 'created_at' + | 'created_at_desc'; + limit?: number; + offset?: number; +} + // ─── Helper functions ──────────────────────────────────────────────────────── export function parseFields(item: Item): Record { diff --git a/web/src/routes/[username]/[workspace]/settings/+page.svelte b/web/src/routes/[username]/[workspace]/settings/+page.svelte index de10b130..7d9f235a 100644 --- a/web/src/routes/[username]/[workspace]/settings/+page.svelte +++ b/web/src/routes/[username]/[workspace]/settings/+page.svelte @@ -8,6 +8,7 @@ import { parseSchema } from '$lib/types'; import CreateCollectionModal from '$lib/components/collections/CreateCollectionModal.svelte'; import EditCollectionModal from '$lib/components/collections/EditCollectionModal.svelte'; + import StorageTab from '$lib/components/settings/StorageTab.svelte'; import { collectionStore } from '$lib/stores/collections.svelte'; import { toastStore } from '$lib/stores/toast.svelte'; import { copyToClipboard } from '$lib/utils/clipboard'; @@ -49,6 +50,7 @@ { id: 'general', label: 'General', icon: '\u2699\uFE0F' }, { id: 'members', label: 'Members', icon: '\uD83D\uDC65' }, { id: 'collections', label: 'Collections', icon: '\uD83D\uDCC1' }, + { id: 'storage', label: 'Storage', icon: '\uD83D\uDCBE' }, { id: 'danger', label: 'Danger Zone', icon: '\u26A0\uFE0F' }, ]; let validTabIds = $derived(tabs.map(t => t.id)); @@ -677,6 +679,10 @@ /> {/if} + {:else if activeTab === 'storage'} +
+ +
{:else if activeTab === 'danger'}