fix(store): workspace list freshness reflects item activity (BUG-1481) (#559)

`pad workspace list` was showing `workspaces.updated_at`, which only
moves on workspace-row mutations (rename, settings, members). After
this fix the effective UpdatedAt surfaces item activity inside the
workspace — answering "where is work happening?" instead of "when was
this row last UPDATEd?".

Implementation: scalar `MAX(items.updated_at)` subquery in
`ListWorkspaces` and `GetUserWorkspaces`, then `effectiveWorkspaceUpdatedAt`
picks the later of the two timestamps (portable across SQLite +
Postgres, no GREATEST). Read-time approach per the bug's design notes.

Visibility-aware: codex review surfaced that a naive MAX leaks activity
timing for items the caller can't see. The member subquery mirrors
`VisibleCollectionIDs` (`collection_access='all'` short-circuits; for
`'specific'` members, system collections + `member_collection_access`
+ `collection_grants` + `item_grants` gate visibility). The guest
subquery limits MAX to items reachable via `collection_grants` /
`item_grants`. All grant lookups are workspace-scoped for
defense-in-depth.

Four regression tests cover: admin `ListWorkspaces`, member
all-access, member specific-access leak guard, and guest leak guard.
This commit is contained in:
xarmian
2026-05-15 12:30:07 -04:00
committed by GitHub
parent d3bd1958c5
commit be68292e03
3 changed files with 404 additions and 9 deletions
+297
View File
@@ -7,6 +7,7 @@ import (
"path/filepath"
"sync"
"testing"
"time"
"github.com/PerpetualSoftware/pad/internal/models"
"github.com/google/uuid"
@@ -365,6 +366,302 @@ func TestWorkspaceCRUD(t *testing.T) {
}
}
// TestListWorkspaces_UpdatedAtReflectsItemActivity is the regression
// test for BUG-1481: `pad workspace list` was showing the workspaces
// row's updated_at, which is only bumped by workspace-row mutations
// (rename, settings, members), not by item activity inside the
// workspace. After the fix, the returned UpdatedAt should be the later
// of workspaces.updated_at and MAX(items.updated_at), so the freshness
// signal answers "where is work happening?".
func TestListWorkspaces_UpdatedAtReflectsItemActivity(t *testing.T) {
s := testStore(t)
ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Active"})
if err != nil {
t.Fatalf("CreateWorkspace: %v", err)
}
coll, err := s.CreateCollection(ws.ID, models.CollectionCreate{
Name: "Tasks",
Schema: `{"fields":[{"key":"status","label":"Status","type":"select","options":["open","done"],"default":"open","required":true}]}`,
})
if err != nil {
t.Fatalf("CreateCollection: %v", err)
}
// Capture the workspace's own row-level updated_at as a baseline.
wsRow, err := s.GetWorkspaceBySlug(ws.Slug)
if err != nil {
t.Fatalf("GetWorkspaceBySlug: %v", err)
}
baselineWS := wsRow.UpdatedAt
// Manually backdate the workspace row's updated_at and forward-date
// a new item, so the only way the freshness can read "later" is by
// surfacing the item's timestamp.
pastTS := "2020-01-01T00:00:00Z"
if _, err := s.db.Exec(s.q(`UPDATE workspaces SET updated_at = ? WHERE id = ?`), pastTS, ws.ID); err != nil {
t.Fatalf("backdate workspace: %v", err)
}
item, err := s.CreateItem(ws.ID, coll.ID, models.ItemCreate{Title: "Recent work"})
if err != nil {
t.Fatalf("CreateItem: %v", err)
}
list, err := s.ListWorkspaces()
if err != nil {
t.Fatalf("ListWorkspaces: %v", err)
}
var got *models.Workspace
for i := range list {
if list[i].ID == ws.ID {
got = &list[i]
}
}
if got == nil {
t.Fatalf("workspace %s not in ListWorkspaces result", ws.ID)
}
// The returned UpdatedAt must reflect the item activity, not the
// (now ancient) workspace row mtime.
if !got.UpdatedAt.Equal(item.UpdatedAt) {
t.Errorf("ListWorkspaces UpdatedAt = %v, want item activity %v (workspace row backdated to %v, baseline was %v)",
got.UpdatedAt, item.UpdatedAt, pastTS, baselineWS)
}
if !got.UpdatedAt.After(parseTime(pastTS)) {
t.Errorf("UpdatedAt %v should be after backdated workspace row mtime %s", got.UpdatedAt, pastTS)
}
}
// TestGetUserWorkspaces_MemberFreshness covers the member path of
// BUG-1481: a workspace returned via membership in GetUserWorkspaces
// must surface item activity as its effective UpdatedAt, not just the
// workspace row mtime.
func TestGetUserWorkspaces_MemberFreshness(t *testing.T) {
s := testStore(t)
ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Member-WS"})
if err != nil {
t.Fatalf("CreateWorkspace: %v", err)
}
coll, err := s.CreateCollection(ws.ID, models.CollectionCreate{
Name: "Tasks",
Schema: `{"fields":[{"key":"status","label":"Status","type":"select","options":["open","done"],"default":"open","required":true}]}`,
})
if err != nil {
t.Fatalf("CreateCollection: %v", err)
}
u := createTestUser(t, s, "member@test.com", "Member", "password123")
if err := s.AddWorkspaceMember(ws.ID, u.ID, "editor"); err != nil {
t.Fatalf("AddWorkspaceMember: %v", err)
}
// Backdate the workspace row, then create an item — the freshness
// must reflect the item's timestamp, not the backdated row.
pastTS := "2020-01-01T00:00:00Z"
if _, err := s.db.Exec(s.q(`UPDATE workspaces SET updated_at = ? WHERE id = ?`), pastTS, ws.ID); err != nil {
t.Fatalf("backdate workspace: %v", err)
}
item, err := s.CreateItem(ws.ID, coll.ID, models.ItemCreate{Title: "Member work"})
if err != nil {
t.Fatalf("CreateItem: %v", err)
}
list, err := s.GetUserWorkspaces(u.ID)
if err != nil {
t.Fatalf("GetUserWorkspaces: %v", err)
}
var got *models.Workspace
for i := range list {
if list[i].ID == ws.ID {
got = &list[i]
}
}
if got == nil {
t.Fatalf("workspace %s not in GetUserWorkspaces result", ws.ID)
}
if !got.UpdatedAt.Equal(item.UpdatedAt) {
t.Errorf("GetUserWorkspaces member UpdatedAt = %v, want item activity %v",
got.UpdatedAt, item.UpdatedAt)
}
}
// TestGetUserWorkspaces_GuestFreshnessLimitedToVisibleItems covers the
// guest path of BUG-1481 AND the data-leak guardrail surfaced in
// codex review: a guest's view of a workspace's UpdatedAt must reflect
// ONLY items they can actually see (via collection_grants or
// item_grants). Activity in items they have no grant on must not leak
// through the freshness signal.
func TestGetUserWorkspaces_GuestFreshnessLimitedToVisibleItems(t *testing.T) {
s := testStore(t)
ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Guest-WS"})
if err != nil {
t.Fatalf("CreateWorkspace: %v", err)
}
owner := createTestUser(t, s, "owner@test.com", "Owner", "password123")
if err := s.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil {
t.Fatalf("AddWorkspaceMember owner: %v", err)
}
visibleColl, err := s.CreateCollection(ws.ID, models.CollectionCreate{
Name: "Visible",
Schema: `{"fields":[{"key":"status","label":"Status","type":"select","options":["open","done"],"default":"open","required":true}]}`,
})
if err != nil {
t.Fatalf("CreateCollection visible: %v", err)
}
hiddenColl, err := s.CreateCollection(ws.ID, models.CollectionCreate{
Name: "Hidden",
Schema: `{"fields":[{"key":"status","label":"Status","type":"select","options":["open","done"],"default":"open","required":true}]}`,
})
if err != nil {
t.Fatalf("CreateCollection hidden: %v", err)
}
// Create the guest user and grant them access ONLY to the visible
// collection — they should never see the hidden collection's items.
guest := createTestUser(t, s, "guest@test.com", "Guest", "password123")
if _, err := s.CreateCollectionGrant(ws.ID, visibleColl.ID, guest.ID, "read", owner.ID); err != nil {
t.Fatalf("CreateCollectionGrant: %v", err)
}
// Backdate the workspace row so neither item is being shadowed by
// the row's own mtime.
pastTS := "2020-01-01T00:00:00Z"
if _, err := s.db.Exec(s.q(`UPDATE workspaces SET updated_at = ? WHERE id = ?`), pastTS, ws.ID); err != nil {
t.Fatalf("backdate workspace: %v", err)
}
// Create a visible item first, then a hidden item LATER — so if
// the freshness query ignores the grant filter, it will surface the
// later (hidden) timestamp and the test will catch the leak.
visibleItem, err := s.CreateItem(ws.ID, visibleColl.ID, models.ItemCreate{Title: "Visible work"})
if err != nil {
t.Fatalf("CreateItem visible: %v", err)
}
// Force a measurable gap; RFC3339 stores second-precision so a
// 1.1s sleep guarantees a distinct later timestamp.
time.Sleep(1100 * time.Millisecond)
hiddenItem, err := s.CreateItem(ws.ID, hiddenColl.ID, models.ItemCreate{Title: "Hidden work"})
if err != nil {
t.Fatalf("CreateItem hidden: %v", err)
}
if !hiddenItem.UpdatedAt.After(visibleItem.UpdatedAt) {
t.Fatalf("setup invariant: hidden item should be newer than visible item (visible=%v hidden=%v)",
visibleItem.UpdatedAt, hiddenItem.UpdatedAt)
}
list, err := s.GetUserWorkspaces(guest.ID)
if err != nil {
t.Fatalf("GetUserWorkspaces guest: %v", err)
}
var got *models.Workspace
for i := range list {
if list[i].ID == ws.ID {
got = &list[i]
}
}
if got == nil {
t.Fatalf("guest workspace %s not in GetUserWorkspaces result", ws.ID)
}
if !got.IsGuest {
t.Errorf("expected workspace to be flagged as guest, got IsGuest=%v", got.IsGuest)
}
// The guest's view must show the visible item's timestamp, NOT the
// newer hidden item's timestamp — that would be a freshness leak.
if !got.UpdatedAt.Equal(visibleItem.UpdatedAt) {
t.Errorf("guest UpdatedAt = %v, want visible item activity %v (leak check: hidden item = %v)",
got.UpdatedAt, visibleItem.UpdatedAt, hiddenItem.UpdatedAt)
}
if got.UpdatedAt.Equal(hiddenItem.UpdatedAt) || got.UpdatedAt.After(hiddenItem.UpdatedAt) {
t.Errorf("guest UpdatedAt %v leaks hidden-item activity %v", got.UpdatedAt, hiddenItem.UpdatedAt)
}
}
// TestGetUserWorkspaces_MemberSpecificAccessFreshnessLimitedToVisibleItems
// covers the member-with-restricted-access path of BUG-1481: a member
// with collection_access='specific' must only see freshness from
// collections they can actually access (via member_collection_access,
// system collections, collection_grants, or item_grants). Without
// scoping, the read-time MAX would leak activity timing for hidden
// collections to restricted members.
func TestGetUserWorkspaces_MemberSpecificAccessFreshnessLimitedToVisibleItems(t *testing.T) {
s := testStore(t)
ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Restricted-WS"})
if err != nil {
t.Fatalf("CreateWorkspace: %v", err)
}
visibleColl, err := s.CreateCollection(ws.ID, models.CollectionCreate{
Name: "Visible",
Schema: `{"fields":[{"key":"status","label":"Status","type":"select","options":["open","done"],"default":"open","required":true}]}`,
})
if err != nil {
t.Fatalf("CreateCollection visible: %v", err)
}
hiddenColl, err := s.CreateCollection(ws.ID, models.CollectionCreate{
Name: "Hidden",
Schema: `{"fields":[{"key":"status","label":"Status","type":"select","options":["open","done"],"default":"open","required":true}]}`,
})
if err != nil {
t.Fatalf("CreateCollection hidden: %v", err)
}
// Restricted member: granted access to the visible collection only.
u := createTestUser(t, s, "restricted@test.com", "Restricted", "password123")
if err := s.AddWorkspaceMember(ws.ID, u.ID, "editor"); err != nil {
t.Fatalf("AddWorkspaceMember: %v", err)
}
if err := s.SetMemberCollectionAccess(ws.ID, u.ID, "specific", []string{visibleColl.ID}); err != nil {
t.Fatalf("SetMemberCollectionAccess: %v", err)
}
// Backdate the workspace row so it can't shadow the items.
pastTS := "2020-01-01T00:00:00Z"
if _, err := s.db.Exec(s.q(`UPDATE workspaces SET updated_at = ? WHERE id = ?`), pastTS, ws.ID); err != nil {
t.Fatalf("backdate workspace: %v", err)
}
// Visible item first, then a NEWER hidden item — if the freshness
// query doesn't filter by visibility, the hidden item's timestamp
// leaks through.
visibleItem, err := s.CreateItem(ws.ID, visibleColl.ID, models.ItemCreate{Title: "Visible work"})
if err != nil {
t.Fatalf("CreateItem visible: %v", err)
}
time.Sleep(1100 * time.Millisecond)
hiddenItem, err := s.CreateItem(ws.ID, hiddenColl.ID, models.ItemCreate{Title: "Hidden work"})
if err != nil {
t.Fatalf("CreateItem hidden: %v", err)
}
if !hiddenItem.UpdatedAt.After(visibleItem.UpdatedAt) {
t.Fatalf("setup invariant: hidden item should be newer than visible item (visible=%v hidden=%v)",
visibleItem.UpdatedAt, hiddenItem.UpdatedAt)
}
list, err := s.GetUserWorkspaces(u.ID)
if err != nil {
t.Fatalf("GetUserWorkspaces: %v", err)
}
var got *models.Workspace
for i := range list {
if list[i].ID == ws.ID {
got = &list[i]
}
}
if got == nil {
t.Fatalf("workspace %s not in GetUserWorkspaces result", ws.ID)
}
if !got.UpdatedAt.Equal(visibleItem.UpdatedAt) {
t.Errorf("restricted member UpdatedAt = %v, want visible item activity %v (leak check: hidden item = %v)",
got.UpdatedAt, visibleItem.UpdatedAt, hiddenItem.UpdatedAt)
}
if got.UpdatedAt.Equal(hiddenItem.UpdatedAt) || got.UpdatedAt.After(hiddenItem.UpdatedAt) {
t.Errorf("restricted member UpdatedAt %v leaks hidden-item activity %v", got.UpdatedAt, hiddenItem.UpdatedAt)
}
}
func TestWorkspaceUniqueSlug(t *testing.T) {
s := testStore(t)
+78 -6
View File
@@ -354,15 +354,55 @@ func (s *Store) ListSystemCollectionIDs(workspaceID string) ([]string, error) {
// GetUserWorkspaces returns all workspaces a user has access to,
// sorted by the user's custom sort order (then name as tiebreaker).
func (s *Store) GetUserWorkspaces(userID string) ([]models.Workspace, error) {
// BUG-1481: include MAX(items.updated_at) so the workspace's
// effective UpdatedAt reflects item activity, not just row mtime.
// For members with collection_access='specific', restrict MAX to
// items in collections the member can actually see (via
// member_collection_access, system collections, collection_grants,
// or item_grants) — otherwise the freshness signal leaks activity
// for collections the member doesn't have access to. Members with
// 'all' (or empty) access see every collection, so the predicate
// short-circuits and behaves like an unrestricted MAX. The
// visibility rule mirrors VisibleCollectionIDs above.
rows, err := s.db.Query(s.q(`
SELECT w.id, w.name, w.slug, w.owner_id, COALESCE(ou.username, ''), w.description, w.settings, w.created_at, w.updated_at, w.deleted_at,
wm.sort_order
wm.sort_order,
(
SELECT MAX(i.updated_at) FROM items i
JOIN collections c ON c.id = i.collection_id
WHERE i.workspace_id = w.id
AND c.workspace_id = w.id
AND i.deleted_at IS NULL
AND c.deleted_at IS NULL
AND (
COALESCE(wm.collection_access, '') IN ('', 'all')
OR c.is_system = ?
OR EXISTS (
SELECT 1 FROM member_collection_access mca
WHERE mca.workspace_id = w.id
AND mca.user_id = wm.user_id
AND mca.collection_id = i.collection_id
)
OR EXISTS (
SELECT 1 FROM collection_grants cg
WHERE cg.workspace_id = w.id
AND cg.collection_id = i.collection_id
AND cg.user_id = wm.user_id
)
OR EXISTS (
SELECT 1 FROM item_grants ig
WHERE ig.workspace_id = w.id
AND ig.item_id = i.id
AND ig.user_id = wm.user_id
)
)
)
FROM workspaces w
JOIN workspace_members wm ON wm.workspace_id = w.id
LEFT JOIN users ou ON ou.id = w.owner_id
WHERE wm.user_id = ? AND w.deleted_at IS NULL
ORDER BY wm.sort_order ASC, w.name ASC
`), userID)
`), s.dialect.BoolToInt(true), userID)
if err != nil {
return nil, fmt.Errorf("get user workspaces: %w", err)
}
@@ -373,15 +413,17 @@ func (s *Store) GetUserWorkspaces(userID string) ([]models.Workspace, error) {
var ws models.Workspace
var createdAt, updatedAt string
var deletedAt *string
var lastItemActivity sql.NullString
if err := rows.Scan(
&ws.ID, &ws.Name, &ws.Slug, &ws.OwnerID, &ws.OwnerUsername, &ws.Description, &ws.Settings,
&createdAt, &updatedAt, &deletedAt,
&ws.SortOrder,
&lastItemActivity,
); err != nil {
return nil, fmt.Errorf("scan workspace: %w", err)
}
ws.CreatedAt = parseTime(createdAt)
ws.UpdatedAt = parseTime(updatedAt)
ws.UpdatedAt = effectiveWorkspaceUpdatedAt(updatedAt, lastItemActivity)
ws.DeletedAt = parseTimePtr(deletedAt)
ws.HydrateDerivedFields()
result = append(result, ws)
@@ -396,8 +438,36 @@ func (s *Store) GetUserWorkspaces(userID string) ([]models.Workspace, error) {
memberIDs[ws.ID] = true
}
// BUG-1481: for guests, the workspace freshness must reflect ONLY
// items the guest can actually see — items in collections they have
// a collection_grant on, or specific items they have an item_grant
// on. Otherwise the workspace's UpdatedAt leaks activity timing for
// items behind grants the guest doesn't hold. The visible-items
// subquery mirrors the WHERE-clause grant logic below.
guestRows, err := s.db.Query(s.q(`
SELECT DISTINCT w.id, w.name, w.slug, w.owner_id, COALESCE(ou.username, ''), w.description, w.settings, w.created_at, w.updated_at, w.deleted_at
SELECT DISTINCT w.id, w.name, w.slug, w.owner_id, COALESCE(ou.username, ''), w.description, w.settings, w.created_at, w.updated_at, w.deleted_at,
(
SELECT MAX(i.updated_at) FROM items i
JOIN collections c ON c.id = i.collection_id
WHERE i.workspace_id = w.id
AND c.workspace_id = w.id
AND i.deleted_at IS NULL
AND c.deleted_at IS NULL
AND (
EXISTS (
SELECT 1 FROM collection_grants cg
WHERE cg.workspace_id = w.id
AND cg.collection_id = i.collection_id
AND cg.user_id = ?
)
OR EXISTS (
SELECT 1 FROM item_grants ig
WHERE ig.workspace_id = w.id
AND ig.item_id = i.id
AND ig.user_id = ?
)
)
)
FROM workspaces w
LEFT JOIN users ou ON ou.id = w.owner_id
WHERE w.deleted_at IS NULL AND (
@@ -413,7 +483,7 @@ func (s *Store) GetUserWorkspaces(userID string) ([]models.Workspace, error) {
WHERE ig.workspace_id = w.id AND ig.user_id = ? AND i.deleted_at IS NULL AND c.deleted_at IS NULL
)
)
`), userID, userID)
`), userID, userID, userID, userID)
if err != nil {
return nil, fmt.Errorf("get guest workspaces: %w", err)
}
@@ -423,9 +493,11 @@ func (s *Store) GetUserWorkspaces(userID string) ([]models.Workspace, error) {
var ws models.Workspace
var createdAt, updatedAt string
var deletedAt *string
var lastItemActivity sql.NullString
if err := guestRows.Scan(
&ws.ID, &ws.Name, &ws.Slug, &ws.OwnerID, &ws.OwnerUsername, &ws.Description, &ws.Settings,
&createdAt, &updatedAt, &deletedAt,
&lastItemActivity,
); err != nil {
return nil, fmt.Errorf("scan guest workspace: %w", err)
}
@@ -434,7 +506,7 @@ func (s *Store) GetUserWorkspaces(userID string) ([]models.Workspace, error) {
continue
}
ws.CreatedAt = parseTime(createdAt)
ws.UpdatedAt = parseTime(updatedAt)
ws.UpdatedAt = effectiveWorkspaceUpdatedAt(updatedAt, lastItemActivity)
ws.DeletedAt = parseTimePtr(deletedAt)
ws.IsGuest = true
ws.HydrateDerivedFields()
+29 -3
View File
@@ -3,6 +3,7 @@ package store
import (
"database/sql"
"fmt"
"time"
"github.com/PerpetualSoftware/pad/internal/models"
)
@@ -12,8 +13,17 @@ import (
// pre-auth/fresh-install bootstrap. End-user workspace switchers should
// call GetUserWorkspaces instead, which scopes to the user's memberships.
func (s *Store) ListWorkspaces() ([]models.Workspace, error) {
// BUG-1481: workspaces.updated_at only moves when the workspace row
// itself changes (rename, settings, members) — it does NOT reflect
// item activity inside the workspace. We surface the latter via
// MAX(items.updated_at) and expose the later of the two as the
// workspace's effective UpdatedAt, so `pad workspace list` answers
// "where is work happening?" rather than "when was this row last
// renamed?". Done in two steps (scalar subquery + Go-side max) to
// stay portable across SQLite (no GREATEST) and Postgres.
rows, err := s.db.Query(s.q(`
SELECT w.id, w.name, w.slug, w.owner_id, COALESCE(ou.username, ''), w.description, w.settings, w.created_at, w.updated_at
SELECT w.id, w.name, w.slug, w.owner_id, COALESCE(ou.username, ''), w.description, w.settings, w.created_at, w.updated_at,
(SELECT MAX(i.updated_at) FROM items i WHERE i.workspace_id = w.id AND i.deleted_at IS NULL)
FROM workspaces w
LEFT JOIN users ou ON ou.id = w.owner_id
WHERE w.deleted_at IS NULL
@@ -28,17 +38,33 @@ func (s *Store) ListWorkspaces() ([]models.Workspace, error) {
for rows.Next() {
var w models.Workspace
var createdAt, updatedAt string
if err := rows.Scan(&w.ID, &w.Name, &w.Slug, &w.OwnerID, &w.OwnerUsername, &w.Description, &w.Settings, &createdAt, &updatedAt); err != nil {
var lastItemActivity sql.NullString
if err := rows.Scan(&w.ID, &w.Name, &w.Slug, &w.OwnerID, &w.OwnerUsername, &w.Description, &w.Settings, &createdAt, &updatedAt, &lastItemActivity); err != nil {
return nil, err
}
w.CreatedAt = parseTime(createdAt)
w.UpdatedAt = parseTime(updatedAt)
w.UpdatedAt = effectiveWorkspaceUpdatedAt(updatedAt, lastItemActivity)
w.HydrateDerivedFields()
workspaces = append(workspaces, w)
}
return workspaces, rows.Err()
}
// effectiveWorkspaceUpdatedAt returns the later of the workspace's own
// updated_at and the most recent item activity inside it. See BUG-1481
// for why item activity is the meaningful freshness signal.
func effectiveWorkspaceUpdatedAt(workspaceUpdatedAt string, lastItemActivity sql.NullString) time.Time {
wsTS := parseTime(workspaceUpdatedAt)
if !lastItemActivity.Valid || lastItemActivity.String == "" {
return wsTS
}
itemTS := parseTime(lastItemActivity.String)
if itemTS.After(wsTS) {
return itemTS
}
return wsTS
}
func (s *Store) CreateWorkspace(input models.WorkspaceCreate) (*models.Workspace, error) {
id := newID()
ts := now()