Generated by a multi-agent deep-analysis workflow: 8 dimensions → adversarial verification → synthesis. 47 findings raised, 37 survived an adversarial refute-pass. Confidence levels and verifier caveats are preserved.
✅ P0 security cluster (S1–S4 / findings #1, #2, #3, #9) is FIXED — committed
e7d2e5dconfeature/communitywithgameplan/tests/test_api_security.py. The warning below is retained for history.
⚠️ Findings #1 and #2 are critical, remotely-exploitable authorization holes. Treat as a security incident — fix before anything else.
This doc is the work-list. Each finding below is independently actionable (has file:line, evidence, fix). A fresh session also auto-loads AGENTS.md + saved memory (conventions, build/test commands, the Cypress-wipes-DB danger, frappe-ui dev symlink) — don't re-derive those.
f723d2f8perf: Sentry deferred to a lazy chunk (entry 120.6→81.3 KB gz; LCP 839→637 ms).08fd8f31perf: motion-v removed fromDiscussionRow+Draftsonly — still used byBottomSheet,ReactionsMobile,RevisionsDialog(so F3/V-refs to those remain valid).6eb98e7drefactor: 47 dead modules deleted +gemojidep removed + dead-routecomponent:lines stripped fromrouter.ts.e7d2e5dcfix(security): P0 cluster S1–S4 done —require_admin()gate +is_admin()helper onchange_user_role/remove_user/invite_by_email;invite_by_emailsplit into gated wrapper + trusted_invite_by_email()(soonboarding+GP Project.invite_gueststill work);get_user_infostripsemailfor guest callers. 9 tests ingameplan/tests/test_api_security.py.3f1aedd7fix(frontend): P1 a11y + V2 + T1 + F3 — aria-labels on raw-<button>controls (RailIcon, ProfileImageEditor, ColorPicker, Search clear); CommandPalette listener leak →useEventListener; typednew_activitysocket payload; lazy-load RevisionsDialog.4c5458e7perf(backend): P1 B4 + B5 done —mark_all_notifications_as_read,clear_notifications,move_to_team→ singleqb.update. Characterization tests ingameplan/tests/test_bulk_updates.py. B6 deferred.8a2b41c9chore: P1 D1–D5 + C3 done — deletedlucideIcons.js,gemoji.py(−9.4k lines), unused exports (noCategories,htmlToText,getScrollParent,joinedCommunities); removedtippy.js/lucide-static/vue-template-compilerdeps (keptvue-virtual-scroller); fixed dead try/catch in Search.vue.
P0 — Security (S1–S4).✅ DONE (e7d2e5dc).require_admin()helper applied acrossremove_user/change_user_role/invite_by_email;get_user_infoscoped for guests. Start at P1.P1 — Quick wins.✅ MOSTLY DONE (3f1aedd7,4c5458e7,8a2b41c9). a11y A2/A3/A5/A8 ✅; A1 icon-Buttonsites (ImagePreview, LastPostReminder, CommunityMembersList) deferred — needs frappe-ui Button fix (it hardcodesaria-labeltoprops.label, clobbering fallthrough attrs; gameplan pins published beta.6). B4/B5 ✅, B6 deferred (shared_notify_user+ enqueue timing change → own PR). F3, V2, T1 ✅. D1–D5, C3 ✅ (vue-virtual-scrollerkept for F4/F5).- P2 — Larger. Editor lazy-load off read path (F1), Options-API migration (V1), comment/query perf (B1, B2, B3).
- frappe-ui submodule boundary: the deep editor trims (highlight.js ~36 langs,
emojis.json) live infrappe-ui/src/— a separate frappe-ui PR + version bump, NOT a gameplan change. Prod builds the published frappe-ui (package.jsonpins1.0.0-beta.6); the local checkout (symlinked,beta.11) only affects local builds. The gameplan-side part of F1 (lazy-loadGPEditor/ render read-only content without mounting TipTap) is doable in this repo. vue-virtual-scrollerconflict: D1 says delete it (unused); F4/F5 say adopt it (People page / feed). Decide before touching — adopting is the higher-value call.- Verify every change:
cd frontend && yarn build(build does NOT type-check — runvue-tsc/IDE separately for type changes). Backend:bench --site gameplan.frappe.test run-tests --app gameplan. Cypress: demo site only (see memory). Re-trace perf via chrome-devtools MCP after abench browse --user Administratorsid login hit on:8000. - Confidence caveats matter: several findings (B1, B2, F5, V4, A2, C1) carry verifier corrections inline — read them before implementing so you don't act on an overstated sub-claim.
| # | Title | Dimension | Severity | Effort | Files |
|---|---|---|---|---|---|
| 1 | ✅ DONE — remove_user endpoint has zero authorization — any user can disable any account |
backend-security | critical | low | gameplan/api.py:101-107 |
| 2 | ✅ DONE — change_user_role lets any member self-promote to Gameplan Admin |
backend-security | critical | low | gameplan/api.py:82-98 |
| 3 | ✅ DONE — invite_by_email/onboarding grant roles with no auth or role allowlist |
backend-security | high | low | gameplan/api.py:110-140, 335-354 |
| 4 | <button>s fixed (3f1aedd7); the 3 real frappe-ui <Button icon> sites need a frappe-ui Button fix (aria-label clobbered by props.label) |
accessibility | high | low | frappe-ui Button.vue:288 + 3 call sites |
| 5 | ✅ DONE — Profile avatar upload button has no accessible name | accessibility | high | low | ProfileImageEditor.vue:10-43 |
| 6 | Full TipTap + highlight.js (284 KB gz) loads eagerly on every discussion read | frontend-perf | high | high | DiscussionView.vue:136,299 + editor chain |
| 7 | Comment insert fans out ~10+ queries + full discussion save per reply | backend-query-perf | high | medium | gp_comment.py:49-57, gp_discussion.py:206-250 |
| 8 | _get_project_members scans all enabled users + is_guest() per user on every post/comment |
backend-query-perf | high | medium | gp_unread_record.py:220-227, __init__.py:6-15 |
| 9 | ✅ DONE — get_user_info (allow_guest) leaks every user's email/roles to any caller |
backend-security | medium | low | gameplan/api.py:13-79 |
| 10 | ✅ DONE — RevisionsDialog (motion-v + htmldiff-js) statically bundled into read path | frontend-perf | medium | low | DiscussionView.vue:301, Comment.vue:109 |
Findings 1, 2, 3 (✅ done), 4, 5, 9 (✅ done), 10, plus: notification bulk-update (B4), move_to_team bulk update (B5), _notify_everyone_mention batching (B6), CommandPalette listener leak (V2), remaining a11y items (A3, A5, A8), typed socket payload (T1), unused deps (D1–D3), gemoji blob (D3). Concentrated, mechanical fixes with outsized payoff.
Why: Any authenticated Gameplan user (incl. Gameplan Guest) can disable any account, including Administrators. DoS / account-takeover-adjacent.
Evidence: api.py:101-107 — @frappe.whitelist() + validate_type only, then user_doc.enabled = 0; user_doc.save(ignore_permissions=True). No role/admin gate. Sibling change_user_role at least checks is_guest(), proving the intended pattern is missing.
Fix: Explicit admin check at the top (System Manager / Gameplan Admin). Build a shared require_admin() helper, reuse across remove_user, change_user_role, invite_by_email.
Confidence: high. (A whitelist without allow_guest does block the unauthenticated Frappe Guest; substance — any authenticated Gameplan user — stands.)
Why: A plain Gameplan Member can call change_user_role(user=<self>, role='Gameplan Admin') and self-promote.
Evidence: api.py:82-98 — only gate is if gameplan.is_guest(): frappe.throw(...); is_guest() (__init__.py:6-15) returns False for any Member/Admin. Then save(ignore_permissions=True). Error string "Only Admin can change user roles" is misleading.
Fix: Gate on admin role explicitly. Same for remove_user, invite_by_email.
Confidence: high.
Status note: core gate + role allowlist shipped. Two report sub-items left as follow-ups (pre-existing behavior, not new escalation): "constrain onboarding to fresh signup" and a space-level permission check inside GP Project.invite_guest.
Why: invite_by_email accepts an arbitrary role string with no permission check, inserts with ignore_permissions=True. Attacker invites an email they control as Gameplan Admin; on acceptance append_roles(self.role) grants admin. Also an unbounded invite/spam vector. onboarding (api.py:335-354) calls it with no gate and freely creates Teams/Projects.
Evidence: api.py:110-140; gp_invitation.py:9-20 (no role allowlist), :53 (append_roles).
Fix: Require admin; validate role against an allowlist (reject Gameplan Admin unless caller is admin). Constrain onboarding to fresh signup.
Confidence: high. (Admin-escalation needs attacker to control + accept the invited address — a real route, not single-call.)
Status note: email is now stripped for guest callers (the report's "minimum" fix). The fuller "scope to accessible communities/projects" option remains open if deeper hardening is wanted.
Why: A restricted Gameplan Guest receives name, email, roles, and 3-month activity for every Gameplan % user — no scoping.
Evidence: api.py:13-79 — @frappe.whitelist(allow_guest=True), only blocks literal session.user == 'Guest'; Has Role via frappe.db.get_all with no permission filter. Caller: data/users.ts:27.
Fix: Scope to accessible communities/projects, or restrict email/role to admins. At minimum drop email for guest callers.
Confidence: high.
Why: Every comment → ~10+ queries + a full doc write; the discussion save() re-runs its entire on_update (notify_mentions, notify_reactions, log_title_update, attach_files_in_content, update_participants_count — computed twice).
Evidence: gp_comment.py:49-57 → update_last_post (gp_discussion.py:216-243), update_post_count (:245-250), update_participants_count (:206-214), track_visit (:108-129), then discussion.save(). Synchronous; no enqueue/debounce.
Fix: Targeted frappe.db.set_value for last_post_*/comments_count/participants_count; one combined aggregate query; decouple track_visit(); avoid re-firing full on_update.
Confidence: high. (Sub-claim that counts "duplicate the unread system" is wrong; doesn't affect the fix.)
Why: For a public space, fetches all enabled GP User Profiles (filtered only on enabled=1) and calls frappe.get_roles(user) per user — scales with whole user base, on every discussion and comment.
Evidence: gp_unread_record.py:220-227, __init__.py:6-15; invoked from gp_discussion.py:76, gp_comment.py:35.
Fix: Resolve guest users in one set query (join GP User Profile to Has Role on Gameplan Guest) and subtract. Cache per (project, request).
Confidence: high. (frappe.get_roles is Redis-cached, so warm-cache cost is N Redis hits, not N DB queries — "N+1" overstates steady state. Fetching the whole user table per event is still real waste.)
Evidence: search_sqlite.py:247-294 (:289-294 tag pull no LIMIT, :300-305 Python count); endpoint api.py:411.
Fix: Combine aggregates; cache per user (short TTL); aggregate tags in SQL or a tag-count table.
Confidence: high.
Evidence: api.py:250-258; gp_notification.py:26-29; called from gp_discussion.py:129.
Fix: Single frappe.qb.update(...).set(read, 1).where(...). Only side effect is an insert hook, not lost.
Confidence: high.
B5. ✅ DONE (4c5458e7) — move_to_team rebuilds every discussion/task one doc at a time (medium, low) — quick win
Evidence: gp_project.py:106-110 — 2N loads + N saves; each save re-runs on_update incl. notify_mentions (spurious re-notification risk).
Fix: Single qb.update/db.set_value per doctype.
Confidence: high.
B6. ⚠️ DEFERRED — _notify_everyone_mention: frappe.db.exists per user in loop (medium, low) — quick win
Status: held out of the P1 batch — touches the shared _notify_user path and the enqueue step changes sync→async timing. Own PR.
Evidence: mentions.py:38-47, :71, :77 — existence SELECT + full doc insert per user, synchronously in on_update.
Fix: Batch the dedup check, bulk_insert, enqueue the fan-out.
Confidence: high.
Why: Largest chunk (GPEditor, 940 KB raw / 284 KB gz, lowlight + ~35 grammars) is statically imported and parsed before any post renders — even for read-only viewers.
Evidence: DiscussionView.vue:299 static import; :136 mounts a real TipTap instance even with :editable=false. Chain: DiscussionViewEditor.vue:4 → GPEditor.vue; CommentsArea.vue:254 → CommentEditor → GPEditor. lowlight via commentExtensions.ts → CommentKit → createLowlight(common) at module top-level.
Fix: Render read-only content via a lightweight HTML renderer; lazy-load editable GPEditor via defineAsyncComponent only when editing/composing. Minimum: lazy-load the lowlight CodeBlock extension.
Confidence: high. Biggest single-chunk win; high effort (no read-only renderer ships in frappe-ui). Lazy-lowlight is the low-risk first step.
Evidence: g.html:233-243; index statically imports a misnamed spaces-*.js (227 KB raw / 64 KB gz headlessui/reka-ui vendor chunk). Root cause: App.vue:9/20 always mounts <NewTaskDialog/> → statically imports Dialog/Dropdown/Combobox/DatePicker. vite.config.ts has no manualChunks.
Fix: defineAsyncComponent for shell-only heavy components (esp. NewTaskDialog); add manualChunks to split the reka-ui/headlessui vendor.
Confidence: high.
F3. ✅ DONE (3f1aedd7) — RevisionsDialog (motion-v + htmldiff-js) statically bundled into read path (medium, low) — quick win
Evidence: DiscussionView.vue:301, Comment.vue:109 static imports; RevisionsDialog.vue:121-123. htmldiff-js has exactly one consumer.
Fix: defineAsyncComponent(() => import('./RevisionsDialog.vue')) in both (pattern already used at Reactions.vue:19, App.vue:24).
Confidence: high.
Evidence: People.vue:200 pageLength: 999; :76 v-for no virtualization; :88 UserAvatarWithHover → reka-ui HoverCard. vue-virtual-scroller already a dep but unused.
Fix: RecycleScroller, or reduce pageLength with incremental loading.
Confidence: high (material only at hundreds+ members).
Evidence: DiscussionList.vue:29-39 (no v-memo), :45-52 (Load more); useList.ts:296 appends.
Fix: v-memo keyed on title/unread/last-activity/selection; and/or RecycleScroller.
Confidence: medium. (The "realtime re-diffs whole list" justification is overstated — useList has no socket sub.)
A1. ⚠️ DEFERRED (frappe-ui) — Icon-only frappe-ui Buttons have no accessible name (high, low) — quick win
Status: raw <button> sites fixed under A2/A3/A5/A8. The 3 real <Button icon> sites (ImagePreview, LastPostReminder, CommunityMembersList) can't be fixed from gameplan — Button.vue:288 hardcodes aria-label: props.label, clobbering fallthrough attrs. Needs a frappe-ui PR + version bump (gameplan pins published beta.6).
Why: WCAG 4.1.2 across ~10 sites; SR announces "button". The tooltip prop does not fix it (renders aria-describedby, not a name).
Evidence: Button.vue:288 sets aria-label only from props.label; icon marked aria-hidden. Sites: LastPostReminder.vue:17, ImagePreview.vue:8, Notifications.vue:77, ReadmeEditor.vue:61, SpaceRow.vue:6,53, InvitePeople.vue:64, CommunityMembersList.vue:38,65.
Fix: Pass label/aria-label on every icon-only Button; optionally patch frappe-ui Button to fall back to tooltip.
Confidence: high.
Evidence: ProfileImageEditor.vue:10-43. No-image case is nameless; with image announced as the avatar, not the action.
Fix: aria-label="Change profile photo".
Confidence: medium (overlay text uses opacity-0, technically in a11y tree, but fragile; no-image case still nameless).
A3. ✅ DONE (3f1aedd7) — Icon-rail nav buttons rely on tooltip for their name (medium, low) — quick win
Evidence: RailIcon.vue:4-15 (no aria), :17-21 tooltip, :7-11 color-only active. Sibling AppRail buttons already use aria-label.
Fix: :aria-label="label" + :aria-current="isActive ? 'page' : undefined".
Confidence: high.
Evidence: ImagePreview.vue:3-6, 14, 23-43 — no role="dialog"/aria-modal/name, no focus move/restore/trap, <img> no alt.
Fix: Use frappe-ui Dialog / reka-ui DialogRoot; manage focus; add alt.
Confidence: high.
Evidence: ColorPicker.vue:16-25 — name only from :title (raw hex); selection not exposed. IconPicker.vue does it right.
Fix: aria-label (color name/hex), aria-pressed/aria-current; consider listbox/option.
Confidence: high.
Evidence: CoverImage.vue:4-10 (no alt), :14, 146-147 (mouse-only). Also uses Vue 2 destroyed() hook (:149) — won't fire in Vue 3 (latent listener-leak bug).
Fix: Add alt; keyboard reposition affordance; fix destroyed → unmounted.
Confidence: high.
Evidence: Poll.vue:58-77 — plain <button>s, no role="radiogroup"/radio, no aria-checked.
Fix: role="radiogroup" labelled by poll title; options role="radio" + aria-checked; branch on multiple_answers for checkbox semantics.
Confidence: high.
Evidence: Search.vue:25-31 — icon-only, decorative lucide-x, no aria-label.
Fix: aria-label="Clear search".
Confidence: high.
Why: Violates AGENTS.md (<script setup lang="ts"> + Composition; data via useList/useDoc/useCall). Last consumers of the v1 resource plugin.
Evidence: People.vue:154/187-205/229, PersonProfile.vue:155-179/182/212-230, Poll.vue:157-171/179-266, Settings/Members.vue:52-76/99-107, UnsplashImageBrowser.vue:61-69 — plain <script>, this.$resources.*.
Fix: Migrate each to <script setup lang="ts">; type:'list'→useList, type:'document'→useDoc, submit()→useCall.
Confidence: high.
V2. ✅ DONE (3f1aedd7) — CommandPalette registers a keydown listener that is never removed (medium, low) — quick win
Why: App.vue:24-32 swaps Layout reactively at 640px, so crossing the breakpoint remounts CommandPalette and adds a duplicate listener with no cleanup.
Evidence: CommandPalette.vue:583-594 (inline arrow in addEventListener), :600-602 (onBeforeUnmount only hides).
Fix: @vueuse/core useEventListener(window, 'keydown', handler) (auto-cleanup).
Confidence: high.
Evidence: CommentsList.vue:29 & CommentsArea.vue:29 border-blue-600; DiscussionRow.vue:103 bg-amber-600 text-white; Onboarding.vue:140 text-red-600; ReactionsMobile.vue:21 & ReactionsDesktop.vue:41 hover:bg-amber-200/text-amber-700. (Semantic tokens like bg-surface-amber-2 are fine — raw shades are the violation.)
Fix: Swap to semantic tokens per frappe-ui SKILL.md.
Confidence: high (design-consistency, no runtime impact).
Evidence: Search.vue:539-545, Page.vue:230-234, CommandPalette.vue:584, ImagePreview.vue:24-27.
Fix: Extract useKeyboardShortcut(combo, handler) over @vueuse.
Confidence: medium (only the ~3-line lifecycle is duplicated; the impactful leak is V2).
T1. ✅ DONE (3f1aedd7) — Realtime new_activity handlers receive untyped (any) payloads (high, low) — quick win
Why: If the backend renames reference_doctype/reference_name, the comparison silently becomes false — no reload, no TS error. "Comments stop live-updating."
Evidence: CommentsArea.vue:801, CommentsList.vue:416 untyped; socket.ts types only refetch_resource; backend activity.py:36-40 publishes {reference_doctype, reference_name}.
Fix: Define type NewActivityEvent next to RefetchResourceEvent, annotate both handlers; ideally a typed ServerToClientEvents map.
Confidence: high (degrades gracefully, so "high" severity is slightly generous).
Evidence: Search.vue:419-431 — reads off any, discarding GroupedSpaceItem; Number(space.value) fallback is dead (value always string).
Fix: Annotate from the real type; drop any and Number().
Confidence: high.
D1. ⚠️ PARTIAL (8a2b41c9) — Unused npm deps: vue-virtual-scroller, tippy.js, vue-template-compiler (medium, low)
Status: removed tippy.js + vue-template-compiler. Kept vue-virtual-scroller — F4/F5 may adopt it for People/feed.
Evidence: package.json:31 vue-virtual-scroller (0 src imports), :28 tippy.js (transitive frappe-ui dep only), :49 vue-template-compiler (Vue 2 compiler in a Vue 3 app).
Fix: Remove all three (decide F4/F5 first — they recommend adopting vue-virtual-scroller).
Confidence: high.
D2. ✅ DONE (8a2b41c9) — Orphaned lucideIcons.js + redundant lucide-static devDependency (medium, low)
Evidence: frontend/lucideIcons.js — only importer of lucide-static, itself never imported. Icon resolution owned by frappe-ui's lucideIconsPlugin.js.
Fix: Delete lucideIcons.js; remove lucide-static.
Confidence: high.
D3. ✅ DONE (8a2b41c9) — gemoji.py — 9,407-line emoji blob used only for one demo-seed call (medium, low)
Evidence: gemoji.py (9,407 lines); only importer demo/team_projects.py:7, used once at :219.
Fix: Replace get_random_gemoji() with random.choice over a short tuple; delete gemoji.py.
Confidence: high.
Evidence: groupedSpaces.ts:67 noCategories (0 external); communities.ts:46 joinedCommunities (only its alias activeCommunities is used).
Fix: Delete noCategories; collapse joinedCommunities into activeCommunities.
Confidence: high.
D5. ✅ DONE (8a2b41c9) — Unused utility exports: htmlToText, getScrollParent in utils/index.ts (low, low)
Evidence: utils/index.ts:16, :47 — 0 external callers; getScrollParent superseded by utils/scrollContainer.ts.
Fix: Delete both.
Confidence: high.
C1. Community/Space vs Team/Project/Category naming drift bleeds into UI strings + types (medium, medium)
Evidence: DiscussionView.vue:218 "Pin to Community" label but local pinToCategory submitting pin_scope:'Category' (:236); ChangeSpaceCategoryDialog.vue titled "Change Community"; groupedSpaces.ts:31-32 "Uncategorized" + noCategories. Wire value 'Category' is correctly schema-bound.
Fix: Settle on Community in UI strings + frontend symbols; rename noCategories→noCommunities, ChangeSpaceCategoryDialog.vue→ChangeSpaceCommunityDialog.vue; document 'Category' == Community on the wire.
Confidence: medium (partly cosmetic; filename/var/label-mismatch are the concrete issues).
C2. ~60 lines of community-selection logic duplicated between ManageCommunitiesDialog.vue and MobileHome.vue (medium, medium)
Evidence: Dialog 105-122/124-156, MobileHome 112-129/131-152 — same selectedCommunityNames, updateJoinedTeams useCall, toggles, toasts. save() tails already diverge (drift risk).
Fix: Extract useCommunitySelection() in src/data/.
Confidence: high.
C3. ✅ DONE (8a2b41c9) — Misleading try/catch around Number() that can never throw (low, low) — quick win
Evidence: Search.vue:397-405 — Number() returns NaN, never throws; catch is dead, inserts a NaN Map key for non-numeric input.
Fix: Drop the try/catch; guard with Number.isNaN.
Confidence: high.
- Decide
vue-virtual-scrollerfirst: D1 says delete it; F4/F5 say adopt it. Adopting it for People/feed is the higher-value call. - Shared admin gate: ✅ DONE —
require_admin()+is_admin()built and applied across S1–S3 ine7d2e5dc. - Shared keyboard composable: V2 + V4 both point at
useKeyboardShortcut/useEventListener— one composable fixes leak + duplication. - Lowest-confidence findings to scrutinize: F5, V4, A2, C1 (caveated above); B1/B2 are high-confidence but each has one inaccurate sub-claim that doesn't affect the core fix. All others high-confidence.