[PM-25884] Disable phishing detection if safari is detected

[Banrion]

🎟️ Tracking

PM-25884

📔 Objective

This pull request disables phishing detection in Safari. This is due to the extension failing to load when phishing detection is enabled in Safari. Safari support will be added at a later release due to the differences in implementation necessary for enabling in Safari.

Note: Test cases are not added at the moment but will be after refactoring the PhishingDetectionService init in PM-29549. This is known tech debt

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude[bot]]

Claude finished @Banrion's task in 3m 16s —— View job

---

Code Review Complete

I've reviewed the changes in this PR that disable phishing detection for Safari browsers. Here's my assessment:

Overview

The implementation correctly disables phishing detection for Safari due to extension loading issues. The change is minimal and focused, which reduces risk.

Key Findings

⚠️ Documentation Gap (phishing-detection.service.ts:115)

The comment "platform limitations" is too vague. Future maintainers won't understand what can or can't be fixed. Consider expanding to:

// Phishing detection is unavailable on Safari due to WebExtension API limitations
// that cause the extension to fail loading when phishing detection is enabled.
// Tracked in PM-25884. Safari support requires architectural changes (PM-29549).

♻️ Test Coverage (phishing-detection.service.spec.ts:84-86)

Codecov shows 0% coverage for the new Safari detection logic (3 lines). While PM-29549 will address this during refactoring, a basic test could prevent regression in the meantime.

🎨 Minor Efficiency Note (phishing-detection.service.ts:116)

The Safari check runs unconditionally during initialization, even when the feature flag is disabled. Moving it inside the switchMap would be slightly more efficient, but the current approach is clearer and the performance impact is negligible.

❓ User Experience

Safari users with premium/enterprise subscriptions won't have phishing protection or any UI indication that the feature is unavailable. Consider documenting this limitation or showing status in settings.

Security Analysis

✅ The security implications are acceptable:

• Feature is behind a feature flag
• Safari extension stability is prioritized appropriately
• Worst-case impact of disabled phishing detection is limited to Safari users
• The limitation is logged for debugging

Verdict

Approved with suggestions - The implementation is correct and safe to merge. The documentation gap and missing tests should be addressed, but neither blocks this PR given the feature flag protection and tracked follow-up work in PM-29549.

---

[github-actions[bot]]

Checkmarx One – Scan Summary & Details – 0903431e-e1a2-41ed-91ae-912e70472bfe

Great job! No new security vulnerabilities introduced in this pull request

[codecov[bot]]

Codecov Report

:x: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 41.89%. Comparing base (ac23878) to head (0639187). :warning: Report is 17 commits behind head on main. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...g-detection/services/phishing-detection.service.ts	0.00%	2 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #17655      +/-   ##
==========================================
- Coverage   41.90%   41.89%   -0.01%     
==========================================
  Files        3591     3591              
  Lines      104159   104161       +2     
  Branches    15710    15712       +2     
==========================================
- Hits        43649    43641       -8     
- Misses      58648    58657       +9     
- Partials     1862     1863       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.