Skip to content

feat: Project & Group Visibility (private/shared + two-level inheritance + UX fixes) - #308

Open
tonyhuangai2026 wants to merge 24 commits into
Chorus-AIDLC:mainfrom
tonyhuangai2026:feat/project-visibility
Open

feat: Project & Group Visibility (private/shared + two-level inheritance + UX fixes)#308
tonyhuangai2026 wants to merge 24 commits into
Chorus-AIDLC:mainfrom
tonyhuangai2026:feat/project-visibility

Conversation

@tonyhuangai2026

Copy link
Copy Markdown

Project Visibility (Private / Shared) + Two-Level Group Inheritance

This branch contains two stacked features: (1) per-project visibility, and (2) per-project-group visibility that projects inherit via a dynamic union. They share the project-access.ts authz core and ship together. A mid-stream regression (empty groups vanishing from the list) was also fixed (commit 4500008).


Part 1 — Project Visibility (Private / Shared)

Summary

Adds a second access dimension to projects on top of multi-tenancy. A project is now either:

  • shared — visible to the whole company (the historical behavior), or
  • private — visible only to its owner and an explicit member list (users and agents).

Membership — not the permission bitset — is what grants access to a private project. Holding project:admin does not bypass it; only the super admin platform role sees everything (for governance).

New projects default to private (owner = creating actor, who is auto-added as the first member). A data migration sets all pre-existing projects to shared, so no current work becomes inaccessible.

Why

Previously every project was visible to everyone in the company (services scoped only by companyUuid). Teams asked for private workspaces that a subset of people/agents can collaborate in.

How it works

A single authz module — src/lib/authz/project-access.ts — is the source of truth:

  • getAccessibleProjectUuids(auth) → the set of project UUIDs the actor may see (or an ALL sentinel for super admin)
  • canAccessProject(auth, projectUuid) → read and write gate
  • canManageProject(auth, projectUuid) → owner-only gate (visibility / membership / delete)
  • applyProjectFilter(where, accessible) → injects projectUuid: { in: [...] } into existing (company-scoped) queries

Access is enforced across the whole cascade — the project and all of its ideas, proposals, documents, tasks, activity, comments, notifications, and search results are filtered for non-members. Both reads and writes are gated (e.g. a non-member agent cannot claim/update a private task or post a comment on it).

Surfaces changed

  • Schema: Project.visibility / ownerType / ownerUuid, new ProjectMember table, migration with shared backfill.
  • Services: project, project-group, idea, proposal, document, task, activity, notification, comment, search, assignment, idea-tracker — all gated.
  • REST API: GET/POST /api/projects, GET/PATCH/DELETE /api/projects/[uuid], new /api/projects/[uuid]/members (GET/POST/DELETE, owner-only), and canAccessProject guards on every nested route. Leak rule: inaccessible → 404, accessible-but-not-owner manage → 403.
  • MCP tools: chorus_admin_create_project gains visibility + memberUuids; new chorus_list_project_members (project:read), chorus_admin_add_project_member / chorus_admin_remove_project_member (project:admin); list/get project & group tools and every projectUuid-taking tool gated.
  • Frontend: Lock badge on private projects; project settings modal gains a visibility toggle + owner-only members manager (shadcn-only, i18n en/zh, IME-safe).
  • Docs: docs/MCP_TOOLS.md + both skill doc sets.

Testing

  • Unit tests for the authz core (full actor × visibility matrix, incl. project:admin-non-member denied and projectUuids[] header does not grant access).
  • Read- and write-gating tests across every affected service.
  • A dedicated end-to-end privacy integration test (src/__tests__/integration/project-visibility.integration.test.ts) that drives the real authz + services over an in-memory Prisma and asserts the full boundary: non-member deny (reads + writes), owner/member allow, super_admin all-access, project:admin-non-member deny, shared-project regression.
  • Full gate green: tsc ✓, pnpm test (1889 pass / 1 skip) ✓, coverage 95.06% stmts / 87.94% branches / 95.95% funcs / 96.82% lines (≥ thresholds) ✓, pnpm build ✓.

Migration / rollout

The migration adds the columns (default private) and runs UPDATE "Project" SET visibility='shared' for all pre-existing rows, so production data stays fully visible. The standalone Docker entrypoint runs prisma migrate deploy automatically on container start.

Known follow-ups (out of scope)

  • Per-member roles (viewer/editor/admin) — currently a single member role.
  • project_group entity names are still searchable in global search (group containers aren't visibility-gated); private-project entities never leak.
  • docs/design.pen not updated in this environment (Pencil MCP tooling unavailable) — to refresh when design tooling is available.

🤖 Generated with Claude Code


Part 2 — Two-Level Visibility (ProjectGroup → Project inheritance)

Summary

Project groups gain the same shared/private + owner + member model, and a project's effective access becomes the dynamic union of its own accessors and its group's accessors.

  • Inheritance = dynamic union: a project's accessors = (project owner + members) ∪ (its group's owner + members). Add someone to a private group → they instantly reach every project in it (and all cascaded entities). No snapshot; computed at query time.
  • "项目级 > 项目组" (project-level is authoritative): a project's own visibility flag wins. A shared project inside a private group is still company-wide; a private project inside a shared group is still restricted — a shared group never exposes its private projects. (Enforced by using only owner/member groups for the project-union, never shared groups.)
  • New groups default private (creator = owner + first member); existing groups migrate to shared. A new project created with a groupUuid defaults to its group's visibility.
  • Group management (visibility, members, update, delete) is owner + super-admin only — no project:admin bypass.

Surfaces

  • Schema: ProjectGroup.visibility/ownerType/ownerUuid, new ProjectGroupMember table, migration with shared backfill.
  • Authz core (project-access.ts): getAccessibleProjectUuids + canAccessProject fold in owned/member groups; new getAccessibleGroupUuids / canAccessGroup / canManageGroup. Two distinct group-sets kept rigorously separate (project-union = owner/member only; group-visibility = shared∪owned∪member).
  • Service / REST: group visibility + member CRUD, listProjectGroups gated by canAccessGroup (preserving the empty-group fix), new /api/project-groups/[uuid]/members, project inherits group visibility default.
  • MCP: chorus_admin_create_project_group gains visibility/memberUuids; new chorus_list_project_group_members (project:read), chorus_admin_add/remove_project_group_member (project:admin); update/delete group tools now owner-gated.
  • Frontend: Lock badge on private groups; manage-group dialog visibility toggle + owner-only members manager (i18n en/zh).
  • Docs: MCP_TOOLS.md + both skill doc sets.

Testing (whole branch)

  • Authz unit matrix extended for group inheritance incl. both cross-case invariants (shared-in-private-group still company-wide; private-in-shared-group still restricted) and project:admin-non-member-denied.
  • End-to-end integration test extended: a group member gains read+write across the group's private project + cascade purely via group membership; dynamic revocation (remove from group → access flips); non-member + project:admin-non-member denied.
  • Full gate green: tsc ✓, 1954 tests pass / 1 skip ✓, coverage 95.16% stmts / 88.5% branches / 96.03% funcs / 96.92% lines (≥ thresholds) ✓, pnpm build ✓.

Migration / rollout

Both migrations (add_project_visibility, add_project_group_visibility) run automatically via the Docker entrypoint and backfill existing rows to shared. Already deployed to the live standalone instance for validation.

Known follow-ups (out of scope)

  • Per-member roles (viewer/editor/admin) — single member role for both projects and groups.
  • Project cannot NARROW/remove inherited group members (union only — by design).
  • Nested groups (single level, unchanged).
  • docs/design.pen not updated (Pencil MCP tooling unavailable in this environment).

Part 3 — Visibility UX fixes (claim-on-manage, group DELETE gate, member UX)

Addresses three user-reported bugs, all rooted in legacy null-owner entities (the migration set pre-existing projects/groups to shared with no owner, so isOwner was false for everyone → manage controls hidden).

  • Claim-on-first-manage: the first actor who can access AND manage an owner-less project/group (via any manage action — set visibility, add/remove member, update, delete) claims ownership and is seeded as a member. Strictly access-gated (a non-member of a private owner-less entity can never claim it — no privacy hole), never reassigns an existing owner, race-safe (guarded updateMany where ownerUuid:null + lost-race re-read), and super_admin manages without claiming.
  • Security fix: DELETE /api/project-groups/[uuid] was ungated — added canAccessGroup(→404) then claimOrCanManageGroup(→403), matching PATCH.
  • Member UX: member lists now resolve display names (getActorName) instead of raw UUIDs; the member-add search surfaces human users (and a default list on empty input) via a forMembers flag on /api/mentionables (the @mention autocomplete default is unchanged).
  • Dashboards show manage controls for claimable legacy entities via a pure canManageOrClaimable* predicate (no write on a GET; the real claim happens on the manage action).

Pure read gates (canManageProject/Group) stay side-effect-free; all MCP mutating tools + REST manage routes use the claim-aware variant.

Testing

  • Authz unit matrix extended: access-gated claim (non-member of private owner-less → denied, no write), never-reassign, lost-race, super_admin no-claim, pure-predicate no-write.
  • E2E integration: legacy null-owner project & group claimed by first accessible manager (owner set + member seeded), a different user then denied, private owner-less non-member denied (owner stays null), super_admin no-claim.
  • Full gate green: tsc ✓, 1990 tests pass / 1 skip ✓, coverage 95.23% stmts / 88.77% branches / 96.06% funcs / 96.97% lines ✓, pnpm build ✓.

Migration / rollout

No schema change (owner columns already exist). Pure logic/UX + the DELETE gate fix. Already deployed to the live standalone instance.

Ubuntu and others added 24 commits June 11, 2026 15:27
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…etAccessibleProjectUuids)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ility; member CRUD

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ask services (reads + writes)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mment/search/assignment/tracker

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e guards

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ct guards, permission-map

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ager + i18n

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hz + in-memory prisma)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… baselines, write-gate coverage

- Document project visibility + member tools in docs/MCP_TOOLS.md and both skill doc sets
- Update permission parity baselines for the 3 new member tools
- Add write-gate rejection tests (task + document services) to keep coverage >= thresholds
- Remove unused imports in idea.service

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… group list

Regression from the visibility cascade: listProjectGroups filtered out any group
with zero accessible projects for non-super-admins, so a freshly created (empty)
group vanished from the list — the UI 'create group' button appeared to do nothing.
Groups are organizational containers (consistent with search treating project_group
as ungated); surface all company groups, with projectCount reflecting only
accessible projects. Adds a regression test for the regular-user empty-group case.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ojects into accessible union + group helpers

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ting + project visibility inheritance

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… + owner-gate group update/delete

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… + i18n

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… service-test group mocks, coverage

- Add dynamic group-inheritance E2E cases (group member read+write via group, dynamic revocation, shared-in-private-group invariant) to project-visibility integration test
- Document two-level inheritance + group member tools in MCP_TOOLS.md and both skill doc sets
- Add projectGroup/projectGroupMember mocks to search/notification/checkin/idea-tracker service tests (getAccessibleProjectUuids now consults groups)
- Add group-helper guard-branch tests to keep coverage >= thresholds

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mable predicates

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ted group DELETE

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (mentionables includeUsersOnEmpty)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ember names + user search in dialogs

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s, coverage guard tests

- E2E: legacy null-owner project/group claimed by first accessible manager; non-member of private owner-less denied; already-owned not reassigned; super_admin no-claim
- Docs: claim-on-first-manage + member names in MCP_TOOLS + both skill docs
- Added claim/predicate guard-branch tests to keep coverage >= thresholds

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

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 96.97% (🎯 95%) 2759 / 2845
🔵 Statements 95.23% (🎯 95%) 3056 / 3209
🔵 Functions 96.06% (🎯 93%) 537 / 559
🔵 Branches 88.77% (🎯 85%) 1969 / 2218
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/lib/authz/project-access.ts 98.73% 98.4% 100% 100% 200, 407
src/services/activity.service.ts 100% 100% 100% 100%
src/services/checkin.service.ts 100% 100% 100% 100%
src/services/comment.service.ts 98.94% 88.73% 93.33% 98.88% 337
src/services/document.service.ts 89.02% 89.65% 84.61% 92.95% 157, 205-216, 308-312, 328-331, 370, 400, 401
src/services/idea-tracker.service.ts 96.73% 85.89% 100% 98.71% 24, 208, 289
src/services/idea.service.ts 92.66% 84.44% 95.12% 96.49% 212-213, 277, 318, 320, 379-381, 388, 434, 435, 465, 523, 555, 599, 613, 637, 638, 679, 689, 848, 858, 1049, 1121
src/services/mention.service.ts 95.69% 79.62% 77.77% 96.51% 366-390, 426
src/services/notification.service.ts 88.67% 88.23% 85.71% 88.23% 446-516
src/services/project-group.service.ts 98.26% 97.75% 100% 100% 243, 388
src/services/project.service.ts 98.11% 96.92% 100% 100% 120, 492
src/services/proposal.service.ts 93.37% 87.59% 98.14% 95.87% 253, 672, 731, 732, 792, 878, 881, 894, 924-936, 957, 1041, 1042, 1071, 1072, 1130, 1176, 1212, 1258, 1300, 1352, 1389, 1423
src/services/search.service.ts 98.4% 93.58% 94.73% 99.13% 57, 467
src/services/session.service.ts 98.16% 96.42% 100% 100% 67, 531
src/services/task.service.ts 88.35% 82% 87.93% 91.13% 310, 313, 328-343, 349-353, 355-359, 501-503, 510, 559, 560, 616, 632, 665, 666, 698, 699, 715-762, 1018, 1021, 1042, 1092, 1206, 1364
Generated in workflow #729 for commit 06f7208 by the Vitest Coverage Report Action

@ChenNima ChenNima left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Request changes

Solid core design — project-access.ts is clean, and the two key invariants hold (the two group-sets are kept rigorously separate, and applyProjectFilter treats an empty accessible set as "match nothing", not "no filter"). The service-layer mainline (project / group / idea / proposal / document / task / activity / comment / checkin / session / idea-tracker / notification.list / search-for-entities) is consistently gated.

However, the "thread auth everywhere" sweep missed a few surfaces, and those happen to be real cross-boundary read/write paths into private projects. Two of them are direct content leaks. Details below, most severe first. I verified each by reading the source.


🔴 BLOCKER 1 — Elaboration subsystem has no visibility gate (non-member read + write into a private project's idea)

src/services/elaboration.service.ts was not touched by this PR and scopes only by companyUuid — no canAccessProject.

Exposed through:

  • chorus_get_elaboration (src/mcp/tools/public.ts:659) — a public tool, no permission gate — calls getElaboration({ companyUuid, ideaUuid }).
  • chorus_answer_elaboration (public.ts:621) — also a public tool. answerElaboration (elaboration.service.ts:136) only checks the round is pending_answers; it does not check canAccessProject, nor even that the caller is the idea's assignee.
  • chorus_pm_start_elaboration / validate / skip (pm.ts) — gated only on the idea:write/idea:admin permission bit, which per this PR's own semantics does not widen access to a private project.

Exploit: any company actor (via the two public tools, even an agent with zero resource permissions) who knows/guesses an ideaUuid in a private project they're not a member of can read the full elaboration (rounds, questions, answers, answerer identities) and write answers that advance the idea's lifecycle. Contrast chorus_get_idea, which is correctly gated via getIdea(..., auth).

Fix: add auth to the elaboration service functions and gate on canAccessProject(auth, idea.projectUuid) at entry; thread auth from the MCP tools.


🔴 BLOCKER 2 — getProposalsByIdeaUuid + the two report server actions leak a private project's proposals and report documents

getProposalsByIdeaUuid(companyUuid, projectUuid, ideaUuid) (src/services/proposal.service.ts:490) queries by { projectUuid, companyUuid } only, returns full proposals (incl. documentDrafts / taskDrafts), and has no auth param. Its callers don't gate either (src/app/(dashboard)/projects/[uuid]/dashboard/panels/actions.ts):

  • getProposalsForIdeaAction (L99)
  • getReportsForIdeaAction (L141) — also feeds approved-proposal uuids into the equally ungated listDocumentsByProposalUuids (document.service.ts:152).

Exploit: a non-member who supplies a private project's projectUuid + any ideaUuid reads all of that project's proposals (full draft content) and report documents.

Telling detail: the adjacent getTasksForProposalAction (same file, L116) correctly threads auth into listTasks({ ..., auth }) — so these two are an omission, not intentional.

Fix: add auth + canAccessProject to getProposalsByIdeaUuid (and listDocumentsByProposalUuids), or gate at both server-action entries.


🔴 BLOCKER 3 — SSE /api/events streams every private project's change events to any company user

src/app/api/events/route.ts (not touched by this PR) filters only by event.companyUuid and an optional client-supplied projectUuid query param — it never calls canAccessProject. RealtimeEvent carries projectUuid / entityType / entityUuid / actorUuid (src/lib/event-bus.ts:11).

Exploit: any authenticated company user opens GET /api/events (no filter, or ?projectUuid=<private>) and receives a live stream of every created/updated/deleted event across every private project in the company, including entity and actor uuids. The module header of project-access.ts claims the boundary cascades to real-time fan-out, but it doesn't. (events/notifications is per-recipient keyed and is fine.)

Fix: gate each event with canAccessProject(auth, event.projectUuid) before sending (cache the accessible set per connection; invalidate on membership change, or fall back to per-event checks).


🟠 HIGH — PATCH /api/projects/[uuid]/group uses an access gate instead of a manage gate, and never gates the destination group

src/app/api/projects/[uuid]/group/route.ts + moveProjectToGroup (project-group.service.ts:339, which does no authz at all).

  • Wrong level: re-parenting a project is gated only by canAccessProject (read+write access). So a plain project member, or any non-owner on a shared project with project:write, can re-group / detach it — this should be owner-only (403), per the PR's stated "group management is owner + super-admin only".
  • Destination not gated: body.groupUuid is checked for company membership only — not canAccessGroup/canManageGroup. An actor can attach a project into a group they neither own nor can access.

Fix: gate source with claimOrCanManageProject (→403); when groupUuid is non-null, require canManageGroup (or at least canAccessGroup) on the destination.


🟡 MEDIUM 1 — Global search leaks private project-group names/descriptions

search.service.ts:457 intersects only the project set with the accessible set; the comment explicitly states "project_group results are unaffected". searchProjectGroups (L373) filters by companyUuid only and never consults getAccessibleGroupUuids.

Exploit: chorus_search / GET /api/search with entityTypes:["project_group"] (or default) returns the uuid + name + description snippet of private groups the actor can't access. chorus_get_project_groups filters correctly via getAccessibleGroupUuids; search bypasses it.

Fix: apply getAccessibleGroupUuids in searchProjectGroups. (Listed as a known follow-up in the PR body, but since name+description leak, I'd treat it as a fix rather than accepted.)


🟡 MEDIUM 2 — Inaccessible task/idea returns 500 instead of 404 on mutation routes (existence oracle)

E.g. tasks/[uuid]/route.ts (PATCH L55, DELETE), tasks/[uuid]/dependencies/*, tasks/[uuid]/claim|release, ideas/[uuid]/route.ts, ideas/[uuid]/claim|release. These pre-check existence with the ungated getTaskByUuid/getIdeaByUuid (company-scoped), so an existing-but-inaccessible entity passes; the service-level gate then throws a plain new Error("…not found"), which withErrorHandler maps to 500 (only ApiError / Prisma P2025 map to 404). Net: nonexistent → 404, existing-but-inaccessible → 500 — the status difference reveals existence. No content leak, but it violates the stated "inaccessible → 404". The idea-move path uses ApiError(...,404) and is the correct pattern to follow.


⚙️ Migrations contain DML (UPDATE) for backfill — violates the "migrations are DDL-only" convention

Both 20260611152319_add_project_visibility and 20260612030604_add_project_group_visibility run UPDATE "Project"/"ProjectGroup" SET visibility='shared'. Project convention is migrations must be DDL-only (no UPDATE/INSERT/DELETE, even for backfill). Please move the backfill to an app-level / one-off step, or confirm an explicit exception with the team.


Minor (non-blocking)

  • memberUuids not validatedaddProjectMember / createProject member seeding (project.service.ts:436, L195-207) writes memberUuid verbatim with no check it's a real actor in the company (cf. chorus_pm_assign_task, which does getAgentByUuid company-scoped). A future uuid collision could silently grant access.
  • getUnreadCount (notification.service.ts:300) isn't visibility-filtered while list is — badge can over-count after access is lost. Recipient-scoped, so not a content leak.
  • chorus_session_checkin_task/checkout_task (session.ts:140) accept any task uuid with no project gate — observability-only cross-boundary write; consider canAccessProject(auth, task.projectUuid).
  • docs/design.pen not updated for the new UI (badges, settings modal, members manager) — required per CLAUDE.md; noted as a known follow-up.

Test coverage gap

The integration test is broad, but happens to miss exactly the leaking surfaces: no elaboration cases, no SSE/eventBus cases, no getProposalsByIdeaUuid/report-action cases, and the search assertions cover task/idea/proposal/document/project but not project_group. Please add regressions for these alongside the fixes.


Nice work overall on the authz core and the cascade through the service layer — the gaps are concentrated in the few files the sweep didn't touch. Happy to re-review once BLOCKER 1–3 and HIGH are addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants