Commit Graph

4 Commits

Author SHA1 Message Date
Marcel
362a84dde9 fix(escapeHtml): cover apostrophe to harden single-quoted attribute use
Sina #5505 action item: escapeHtml escaped the four common entities but
not the apostrophe. Today every consumer uses double-quoted attributes,
but a future renderer change to single quotes would silently open a
stored-XSS hole. Cheaper to fix now, with a regression test.

Also pin the idempotence-by-composition property: a second call
re-escapes the & introduced by the first.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-29 01:09:13 +02:00
Marcel
c7013f4902 refactor(mention): extract shared escapeHtml helper
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-29 00:02:03 +02:00
Marcel
dc6ea080c4 fix(#71-#73): address all review findings from Markus and Sara
BLOCKERs:
- Remove direct AppUserRepository/CommentRepository access from CommentService and
  NotificationService — replaced with UserService.findAllById() and UserService
  (fixes layering contract from CLAUDE.md)
- Switch Optional<JavaMailSender> constructor injection — removes @Autowired(required=false)
  field and ReflectionTestUtils hack in tests
- Add @RequirePermission(READ_ALL) to UserSearchController — prevents user enumeration
  without read access

Data bug:
- Promote actorName from @Transient to persisted VARCHAR column (V18 migration)
- Set actorName in notifyReply and notifyMentions from comment.getAuthorName()

Architecture:
- Add @RequirePermission(READ_ALL) to NotificationController
- Introduce NotificationDTO — controller returns DTO instead of Notification entity,
  eliminating lazy-load N+1 and AppUser field leakage
- Change mentions FetchType to EAGER — fixes LazyInitializationException outside transaction
- Add @Transactional(propagation=REQUIRES_NEW) to notifyReply/notifyMentions so a
  notification failure cannot roll back the parent comment
- N+1 fix: replace per-ID findById loops with single findAllById bulk fetch
- Move collectParticipantIds to CommentService; notifyReply accepts Set<UUID> directly

Security:
- Escape displayName before injecting into renderBody HTML span
- Replace <a href="#"> with <span class="mention"> — no profile page to link to, and
  the anchor's scroll-to-top behaviour is harmful

Tests added/fixed:
- markRead_throwsNotFound, markAllRead_delegatesToRepository, countUnread_delegatesToRepository
- markOneRead_returns401, @RequirePermission 403 coverage for both controllers
- postComment/replyToComment_triggersNotifyMentions_whenMentionedUserIdsProvided
- search_returnsAtMostTenResults now asserts $.length() <= 10
- XSS regression test for escaped displayName in mention.spec.ts

Frontend minors:
- relativeTime() uses Intl.RelativeTimeFormat (locale-aware, not German-hardcoded)
- aria-label uses m.notification_unread() Paraglide key (de/en/es added)
- <div role="button"> replaced with <button> (native Enter+Space handling)
- onDestroy clears debounceTimer in MentionEditor
- setTimeout(100) replaced with await tick() + requestAnimationFrame in CommentThread
- Notification prefs form uses checkbox name attributes + formData.has() pattern

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-28 00:31:38 +01:00
Marcel
55cf1fb0a4 feat(#72): add @mention support in comment editor
- mention.ts: detectMention (cursor-aware), extractContent (parse @Name → UUID), renderBody (XSS-safe: escape-first then inject anchor tags, replaceAll for all occurrences)
- 19 unit tests in mention.spec.ts (all green)
- MentionEditor.svelte: textarea with @-trigger popup, debounced /api/users/search, keyboard navigation (↑↓ Enter Esc), Ctrl+Enter submit, @ button for accessibility
- CommentThread.svelte: replace plain textareas with MentionEditor, send mentionedUserIds on post/reply/edit, render comment bodies with {@html renderBody(...)}
- types.ts: add MentionDTO, add optional mentionDTOs to Comment and CommentReply

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-27 20:32:54 +01:00