[MM-68968] Guard redirects and subframe navigations alongside will-navigate

[devinbinnie]

Summary

Our navigation checks were only registered on will-navigate, which Electron emits for top-level, renderer-initiated navigations. The other navigation events it exposes — will-redirect and will-frame-navigate — were never wired up, so navigations arriving through those paths were never evaluated against the same rules.

This PR registers the existing checks on those events too, covering server views, the Calls widget window, and plugin popups. Redirects are dispatched by frame type so that subframes continue to use the subframe check rather than the main-frame one, since ordinary embeds redirect as a matter of course.

The subframe guard moved into webContentEventsCommon so all three call sites can share a single implementation.

Ticket Link

https://mattermost.atlassian.net/browse/MM-68968

Release Note

Improved handling of page redirects and embedded content navigation across Desktop App windows.

Change Impact: 🟡 Medium

Regression Risk: Changes affect shared navigation handling across multiple Electron windows. Automated tests cover the updated main-frame and subframe paths.

QA Recommendation: Manual QA can be skipped because automated coverage is complete and rollback is straightforward.

Generated by CodeRabbitAI

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3e038738-2e50-4b1a-a5fb-5ab6cd21618e

📥 Commits

Reviewing files that changed from the base of the PR and between 0666b39 and d43ce47.

📒 Files selected for processing (8)

• src/app/callsWidgetWindow.test.js
• src/app/callsWidgetWindow.ts
• src/app/views/pluginsPopUps.test.js
• src/app/views/pluginsPopUps.ts
• src/app/views/webContentEvents.test.js
• src/app/views/webContentEvents.ts
• src/app/views/webContentEventsCommon.test.ts
• src/app/views/webContentEventsCommon.ts

---

📝 Walkthrough

Walkthrough

The change adds shared subframe navigation validation and applies it to redirect and frame-navigation events in calls widgets, plugin popups, and web contents. Tests cover allowed web URLs and blocked custom protocols.

Changes

Navigation handling

Layer / File(s)	Summary
Shared frame-navigation policy src/app/views/webContentEventsCommon.ts, src/app/views/webContentEventsCommon.test.ts	generateWillFrameNavigate allows main-frame and permitted web subframe navigation. It logs and prevents other subframe protocols.
Web contents redirect integration src/app/views/webContentEvents.ts, src/app/views/webContentEvents.test.js	will-redirect routes main-frame redirects through the existing policy and subframe redirects through the shared frame-navigation policy. Listener setup and cleanup are tested.
Widget and popup navigation wiring src/app/callsWidgetWindow.ts, src/app/callsWidgetWindow.test.js, src/app/views/pluginsPopUps.ts, src/app/views/pluginsPopUps.test.js	Calls widgets and plugin popups register frame and redirect handlers. Tests cover permitted web navigation and prevented custom-protocol subframe navigation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WebContents
  participant NavigationHandlers
  participant MainFramePolicy
  participant SubframePolicy

  WebContents->>NavigationHandlers: will-redirect or will-frame-navigate
  NavigationHandlers->>MainFramePolicy: validate main-frame navigation
  NavigationHandlers->>SubframePolicy: validate subframe navigation
  SubframePolicy-->>WebContents: allow web URL or prevent custom protocol

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the PR's main change: adding guards for redirects and subframe navigations alongside will-navigate.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch MM-68968

---

_{Comment @coderabbitai help to get the list of available commands.}

[edgarbellot]

LGTM 👍 main path + edge cases covered, thank you!

[mattermost-build]

Cherry pick is scheduled.

[mattermost-build]

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
From github.com:mattermost/desktop
 * [new branch]        MM-68968    -> upstream/MM-68968
   0666b39f..86d054b2  master      -> upstream/master
   d3e8ed25..0a68dc45  release-6.2 -> upstream/release-6.2
   9e8b96f5..01b8d6e9  release-6.3 -> upstream/release-6.3
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-MM-68968-release-6.3-1785508722
Switched to a new branch 'automated-cherry-pick-of-MM-68968-release-6.3-1785508722'
Branch 'automated-cherry-pick-of-MM-68968-release-6.3-1785508722' set up to track remote branch 'release-6.3' from 'upstream'.

+++ About to attempt cherry pick of PR #3927 with merge commit 86d054b2ed8bd6e5f2194e978f2da9384aa6df5c.

Auto-merging src/app/callsWidgetWindow.test.js
Auto-merging src/app/callsWidgetWindow.ts
CONFLICT (content): Merge conflict in src/app/callsWidgetWindow.ts
error: could not apply 86d054b2... [MM-68968] Guard redirects and subframe navigations alongside will-navigate (#3927)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

+++ Conflicts detected:

UU src/app/callsWidgetWindow.ts
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the master branch and cleaning up.

[devinbinnie]

/cherry-pick release-6.2

[mattermost-build]

Cherry pick is scheduled.

[mattermost-build]

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-MM-68968-release-6.2-1785508741
Switched to a new branch 'automated-cherry-pick-of-MM-68968-release-6.2-1785508741'
Branch 'automated-cherry-pick-of-MM-68968-release-6.2-1785508741' set up to track remote branch 'release-6.2' from 'upstream'.

+++ About to attempt cherry pick of PR #3927 with merge commit 86d054b2ed8bd6e5f2194e978f2da9384aa6df5c.

Auto-merging src/app/callsWidgetWindow.test.js
Auto-merging src/app/callsWidgetWindow.ts
CONFLICT (content): Merge conflict in src/app/callsWidgetWindow.ts
error: could not apply 86d054b2... [MM-68968] Guard redirects and subframe navigations alongside will-navigate (#3927)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

+++ Conflicts detected:

UU src/app/callsWidgetWindow.ts
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the master branch and cleaning up.