e2e: use canonical HTTPS URLs in external fixtures

[cursor]

Summary

Fixes master E2E failures caused by external test fixtures bypassing the HTTPS normalization used by normal Desktop server configuration.

Classification: B — E2E fixture issue.

The affected tests wrote noncanonical external URLs directly into config.json, bypassing the Add Server flow that prefers HTTPS and adopts the validated SiteURL.

This PR now contains only three E2E fixture corrections:

• use https://example.com/ in demoConfig;
• use Google's canonical https://www.google.com/ URL in the tab-order fixture;
• use the direct TLS 1.1 endpoint, https://tls-v1-1.badssl.com:1011.

There are no product (src/) or CI workflow changes in the final diff.

Ticket Link

E2E regression exposed after #3927

Checklist

• Added or updated unit tests (not applicable; fixture-only change)
• Has UI changes
• read and understood our Contributing Guidelines
• executed lint/type checks
• Run E2E tests by adding label E2E/Run

Device Information

Linux (Cursor Cloud), with local Mattermost 11.10.0-rc2 for server-backed coverage.

Screenshots

N/A — no UI changes.

Verification

Targeted local results:

MM-T6129 OAuth deep link             passed
MM-T1312 Add Server startup          passed
MM-T6184 TLS 1.1 handling            passed
MM-T2635_1 original tab order        passed
MM-T2635_2 reordered tabs            passed

Full CI/TSIO:

287 passed
0 failed
0 flaky
11 skipped

Linux, Windows, macOS, both policy suites, CI builds, and TSIO summary passed.

Release Note

NONE

[mattermost-build]

Hello @cursor[bot],

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The navigation handler tracks app-initiated loads and centralizes authorization. Matching same-host HTTP-to-HTTPS upgrades are allowed, while downgrades and mismatched OAuth navigations remain blocked. Windows setup downloads yq directly and retries vcredist-all installation.

Changes

Navigation authorization

Layer / File(s)	Summary
App-initiated load tracking src/app/views/MattermostWebContentsView.ts	The view records active application load URLs, exposes pendingLoadURL, and clears the URL after a successful load.
Centralized navigation authorization src/app/views/webContentEvents.ts	The handler checks approved destinations and matching HTTP-to-HTTPS upgrades. It retains mailto: handling and blocks downgrades.
Upgrade and downgrade coverage src/app/views/webContentEvents.test.js	Tests cover matching and mismatched OAuth navigations, redirects, upgrades, downgrades, and reset view mocks.

Windows setup

Layer / File(s)	Summary
Retried Windows dependency installation .github/actions/install-os-dependencies/action.yaml, .github/workflows/ci.yaml	Windows setup downloads yq v4.20.1 from GitHub with retries and SHA-256 verification. The reusable action retries vcredist-all installation.

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

Sequence Diagram(s)

sequenceDiagram
  participant WebContents
  participant webContentEvents
  participant MattermostWebContentsView
  WebContents->>webContentEvents: Request navigation
  webContentEvents->>MattermostWebContentsView: Read pendingLoadURL
  MattermostWebContentsView-->>webContentEvents: Return app-initiated URL
  webContentEvents-->>WebContents: Allow matching upgrade or block navigation

Loading

Possibly related PRs

• mattermost/desktop#3872: Both PRs modify webContentEvents.ts and its navigation tests.
• mattermost/desktop#3928: Both PRs modify navigation handling and redirect policies.
• mattermost/desktop#3929: Both PRs enforce and test Electron navigation and protocol-transition policies.

Suggested reviewers: devinbinnie, edgarbellot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title relates to HTTPS URL handling but does not clearly describe the main navigation-guard fix.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cursor/e2e-failure-resolution-aa7e

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/app/views/webContentEvents.test.js (1)

132-143: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add cross-host HTTPS regression cases.

The new logic must allow only same-host HTTPS upgrades. Add a cross-host HTTPS assertion to both the direct-navigation and redirect tests. This prevents a future broadening of the internal-URL check from bypassing the navigation boundary.

Also applies to: 197-215

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/views/webContentEvents.test.js` around lines 132 - 143, Extend the
navigation tests around willNavigate and the redirect cases near
generateWillNavigate to cover HTTPS upgrades targeting a different host. Assert
that cross-host HTTPS navigation still calls event.preventDefault, while
preserving the existing same-host upgrade and HTTPS-to-HTTP downgrade
expectations.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/app/views/webContentEvents.test.js`:
- Around line 132-143: Extend the navigation tests around willNavigate and the
redirect cases near generateWillNavigate to cover HTTPS upgrades targeting a
different host. Assert that cross-host HTTPS navigation still calls
event.preventDefault, while preserving the existing same-host upgrade and
HTTPS-to-HTTP downgrade expectations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fe92a704-b392-4255-b4bc-b7569038c7d2

📥 Commits

Reviewing files that changed from the base of the PR and between 86d054b and 53f9d52.

📒 Files selected for processing (2)

• src/app/views/webContentEvents.test.js
• src/app/views/webContentEvents.ts

[cursor]

Runtime proof of the root cause (local, Linux)

The original description inferred the mechanism from CI logs. It has now been confirmed directly by temporarily instrumenting the guard on a build of current master (86d054b, i.e. with this fix reverted):

will-redirect isMainFrame=true url=https://example.com/
Prevented desktop from navigating to external URL target=https://example.com/ server=http://example.com/
failed loading URL: Error: ERR_FAILED (-2) loading 'http://example.com/', retrying in 5 seconds

So the exact mechanism is: Chromium upgrades the main-frame http://example.com/ navigation to https:// (HTTPS-Upgrades/HSTS) and reports it as a main-frame redirect. #3927 routes will-redirect into the will-navigate policy, isInternalURL requires an exact scheme match, so the app cancels its own server load → ERR_FAILED → 5s retry loop → electron.launch times out at 60s. The instrumentation has been removed; no debug code is committed.

Before / after on the same spec (same machine, same command):

build	Prevented desktop from navigating hits	result
guard reverted to 86d054b	12	1 failed — TimeoutError: electron.launch: Timeout 60000ms exceeded
with this fix	0	1 passed (2.1s)

npx playwright test specs/server_management/add_server_modal.test.ts -g MM-T1312 --project=linux --workers=1

Also re-verified: webContentEvents + webContentEventsCommon unit suites → 85 passed; eslint on the touched file reports only the pre-existing TODO warning; tsc --noEmit clean.

Note on HTTP-only servers: the upgrade attempt is now allowed rather than cancelled, so Chromium's automatic fallback to http:// still works — the configured server URL stays http://, so the fallback navigation matches the normal internal-URL check. That is the pre-#3927 behaviour.

Residual scope (deliberately not changed here): the view ends up on https:// while ServerManager still stores http://, so callers that match by exact scheme (preAuthManager, sessionAttributesManager via lookupServerByURL) will not match an upgraded server. Broadening isInternalURL globally would also loosen the inverted popup redirect check in this same file, so the exception is kept local to the navigation guard for this regression fix.

View PR

  

_{Sent by Cursor Automation: Desktop Master E2E Guardian}

[saturninoabril]

I'll defer review to Devin, especially for changes at src/app/views/webContentEvents.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/actions/install-os-dependencies/action.yaml:
- Around line 78-81: Verify every downloaded yq.exe in
.github/actions/install-os-dependencies/action.yaml lines 78-81 and both
download sites in .github/workflows/ci.yaml lines 79-81 and 116-118 against
SHA-256 6e8231e771e36b67ca96ba754fc09bbbba95d08a9149411b36011681e46c4f03,
failing before use when verification fails. At each site, invoke
/c/ProgramData/chocolatey/bin/yq.exe explicitly for the version check.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e1ebc5f5-c323-4806-b1c6-bc0500bde3d4

📥 Commits

Reviewing files that changed from the base of the PR and between d64c45a and aea2552.

📒 Files selected for processing (2)

• .github/actions/install-os-dependencies/action.yaml
• .github/workflows/ci.yaml

[devinbinnie]

@yasserfaraazkhan Looks like the finding here is valid, the will-redirect handler is blocking HSTS upgrades, however I think this is the wrong fix. I was able to reproduce, and I was able to fix the issue by simply adding an exception in the will-redirect handler:

const willRedirect = (event: Event<WebContentsWillRedirectEventParams>, url?: string) => {
    if (event.isMainFrame) {
        if (url) {
            const redirectURL = parseURL(url);
            const serverURL = this.getServerURLFromWebContentsId(contents.id);
            if (serverURL?.toString().replace(/^http:\/\//, 'https://') === redirectURL?.toString()) {
                this.log(contents.id).debug('direct HSTS upgrade detected, allowing redirect');
                return;
            }
        }
        willNavigate(event, url);
    } else {
        willFrameNavigate(event);
    }
};

We should bounce this off of Product Security, but I think this is enough.

[devinbinnie]

How come this change is needed? Does it break the test if we don't include a port?

[devinbinnie]

Same here, just wondering.

[yasserfaraazkhan]

@devinbinnie
both original URLs break their tests after redirects became guarded. These changes avoid testing unrelated redirects from the origin

[mattermost-build]

Cherry pick is scheduled.

[amyblais]

/cherry-pick release-6.2

[mattermost-build]

Cherry pick is scheduled.