Skip to content

fix: security hardening, XSS prevention, perf fixes, and org-based KB visibility#3177

Open
srujan00123 wants to merge 2 commits intofrappe:developfrom
srujan00123:fix/code-review-security-and-quality
Open

fix: security hardening, XSS prevention, perf fixes, and org-based KB visibility#3177
srujan00123 wants to merge 2 commits intofrappe:developfrom
srujan00123:fix/code-review-security-and-quality

Conversation

@srujan00123
Copy link
Copy Markdown

@srujan00123 srujan00123 commented Apr 3, 2026

Summary

Comprehensive code review fixes covering security vulnerabilities, frontend XSS prevention, performance improvements, and a new organization-based knowledge base visibility feature.

Security Fixes

  • Auth logic bug: Fixed admin check that always evaluated truthy + "Admistrator" typo (api/auth.py)
  • Auth bypass: Changed substring path matching to exact prefix matching (auth.py)
  • Permission bypass: has_app_permission() no longer returns True for everyone (api/permission.py)
  • IDOR prevention: Added frappe.has_permission() check in saved replies API
  • Script injection: Escape values with json.dumps() in generated JS (field_dependency.py)
  • Socket.IO: Added socket.user validation and room membership checks on all events
  • Window globals: Restricted to allowlist instead of accepting arbitrary API response keys

XSS Prevention (Frontend)

  • Replaced all v-html with text interpolation or stripHtml() in SearchAgent, SearchArticles, ConfirmDialog, dialogs.jsx

Performance & Stability

  • Fixed N+1 query in doc.py group_by options (batch fetch labels)
  • Fixed event listener memory leak in realtime.ts (stored handler refs + cleanup)
  • Added try-catch around JSON.parse calls in frontend composables
  • Reduced view pageLength from 1000 to 100
  • Added missing return after redirect in router guard
  • Guarded empty count query result in doc.py

Feature: Organization-Based KB Article Visibility

  • Added visibility field (Public/Restricted) to HD Article
  • New HD Article Organization child doctype for org assignment
  • Added organization Link field to HD Customer
  • KB API filters articles by user's organization membership
  • Agents see all articles; customers see Public + their org's Restricted articles

Code Quality

  • Fixed docker init.sh shebang and added set -e
  • Fixed AUTHOR_EMAIl typo in welcome_ticket.py
  • Removed debug print() from 5 patch files
  • Added email validation and duplicate check in agent invites
  • Added pagination to get_users() API
  • Fixed unreachable code and generic error swallowing in knowledge_base.py
  • Fixed redundant f-string+format in hd_ticket.py permission_query
  • Fixed via_customer_portal detection in ticket creation

Test Plan

  • Verify agent login works correctly (admin check fix)
  • Verify portal users can only access allowed endpoints (auth.py fix)
  • Verify saved replies require ticket read permission
  • Verify search results display without XSS (text-only rendering)
  • Verify Socket.IO events only broadcast to room members
  • Verify group-by views load efficiently (no N+1)
  • Test KB visibility: agents see all, customers see Public + their org's Restricted
  • Test restricted article direct URL access returns 403 for unauthorized users
  • Verify bench start runs cleanly
  • Run bench --site <site> migrate to apply doctype changes

Security fixes:
- Fix admin check logic bug (always evaluated truthy) and typo in api/auth.py
- Fix auth bypass via substring path matching in auth.py
- Fix has_app_permission() always returning True
- Add permission check in saved_replies API (IDOR prevention)
- Escape values in generated JS to prevent injection (field_dependency.py)
- Fix bare except clause in hd_ticket.py permission check
- Remove v-html XSS vectors in SearchAgent, SearchArticles, ConfirmDialog, dialogs
- Add socket.user validation and room membership checks in Socket.IO handlers
- Restrict window globals to allowlist in main.js

Performance & stability:
- Fix N+1 query in doc.py group_by options (batch fetch labels)
- Fix event listener memory leak in realtime.ts (stored handler refs + cleanup)
- Add try-catch around JSON.parse calls in frontend composables
- Reduce view pageLength from 1000 to 100
- Add missing return after redirect in router guard
- Guard empty count query result in doc.py

Feature: Organization-based KB article visibility
- Add visibility field (Public/Restricted) to HD Article
- Add HD Article Organization child doctype for org assignment
- Add organization Link field to HD Customer
- Filter KB articles by user's organization membership
- Agents see all articles; customers see Public + their org's Restricted

Code quality:
- Fix Python version requirement (3.14 -> 3.11) in pyproject.toml
- Fix docker/init.sh shebang and add set -e
- Fix AUTHOR_EMAIl typo in welcome_ticket.py
- Remove debug print() from 5 patch files
- Add email validation and duplicate check in agent invites
- Add pagination to get_users() API
- Fix unreachable code and generic error handling in knowledge_base.py
- Fix redundant f-string+format in hd_ticket.py permission_query
- Fix via_customer_portal detection in ticket creation
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.

1 participant