fix: prevent microphone indicator on startup by adding setPermissionCheckHandler

[KanishJebaMathewM]

Fixes #3112

Problem

On Windows, the app triggers the microphone-in-use taskbar indicator
on startup because the webapp's routine navigator.permissions.query()
calls escalated into OS-level getMediaAccessStatus('microphone') calls.

Fix

Added setPermissionCheckHandler in serverView/index.ts that returns
true for 'media' permission checks. This intercepts the webapp's
startup permission queries before they escalate to OS API calls that
trigger the mic indicator.

Summary by CodeRabbit

• Bug Fixes
  • Improved permission handling for guest webviews with more restrictive access controls to essential permissions.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Walkthrough

Added a permission check handler for Electron's session that selectively allows only 'media' permissions while denying others on guest webview sessions when they become ready, preventing unnecessary system resource access on startup.

Changes

Cohort / File(s)	Summary
Permission Check Handler src/ui/main/serverView/index.ts	Added handlePermissionCheck handler that grants only 'media' permissions while denying all others. Handler is registered on guest webview sessions when WEBVIEW_READY fires.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding setPermissionCheckHandler to prevent the microphone indicator on startup.
Linked Issues check	✅ Passed	The code change implements the fix for issue #3112 by intercepting permission checks with setPermissionCheckHandler, preventing OS-level microphone access escalation.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the microphone indicator issue; no unrelated or out-of-scope modifications are present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

src/ui/main/serverView/index.ts (2)

431-434: Consider adding a comment explaining the purpose of the permission check handler.

This handler is specifically added to fix issue #3112 (preventing the Windows microphone indicator on startup). Adding a brief comment would help future maintainers understand why this handler exists and why it differs in scope from setPermissionRequestHandler.

+   // Intercept synchronous permission checks (e.g., navigator.permissions.query())
+   // to prevent OS-level API calls that trigger the Windows microphone indicator on startup.
+   // See: https://github.com/RocketChat/Rocket.Chat.Electron/issues/3112
    guestWebContents.session.setPermissionCheckHandler(handlePermissionCheck);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/main/serverView/index.ts` around lines 431 - 434, Add a brief comment
above the guestWebContents.session.setPermissionCheckHandler(...) call
explaining that handlePermissionCheck is registered specifically to work around
issue `#3112` (prevent the Windows microphone indicator appearing on startup) and
that this check is intentionally broader/different in scope from
guestWebContents.session.setPermissionRequestHandler(handlePermissionRequest)
(which handles per-request permission prompts), so future maintainers understand
why both handlers exist and how they differ.

---

373-376: Simplify the conditional return.

The if/return pattern can be simplified for readability.

✨ Suggested simplification

  const handlePermissionCheck: Parameters<
    Session['setPermissionCheckHandler']
  >[0] = (_webContents, permission) => {
-   if (permission === 'media') {
-     return true;
-   }
-   return false;
+   return permission === 'media';
  };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/main/serverView/index.ts` around lines 373 - 376, Replace the
redundant if/return pattern that checks the permission variable with a single
boolean return; locate the snippet using the permission variable in
serverView/index.ts (inside the function/method that currently contains "if
(permission === 'media') { return true; } return false;") and change it to
directly return the expression permission === 'media' to simplify and improve
readability.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/ui/main/serverView/index.ts`:
- Around line 431-434: Add a brief comment above the
guestWebContents.session.setPermissionCheckHandler(...) call explaining that
handlePermissionCheck is registered specifically to work around issue `#3112`
(prevent the Windows microphone indicator appearing on startup) and that this
check is intentionally broader/different in scope from
guestWebContents.session.setPermissionRequestHandler(handlePermissionRequest)
(which handles per-request permission prompts), so future maintainers understand
why both handlers exist and how they differ.
- Around line 373-376: Replace the redundant if/return pattern that checks the
permission variable with a single boolean return; locate the snippet using the
permission variable in serverView/index.ts (inside the function/method that
currently contains "if (permission === 'media') { return true; } return false;")
and change it to directly return the expression permission === 'media' to
simplify and improve readability.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 14a0cc5e-9347-4aed-a24a-df676f1df31f

📥 Commits

Reviewing files that changed from the base of the PR and between 2e0c3ea and 5dba8b9.

📒 Files selected for processing (1)

• src/ui/main/serverView/index.ts

📜 Review details

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from @rocket.chat/fuselage for all UI work and only create custom components when Fuselage doesn't provide what's needed
Check Theme.d.ts for valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux

Files:

• src/ui/main/serverView/index.ts

🧠 Learnings (1)

📓 Common learnings

Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.

🔇 Additional comments (1)

src/ui/main/serverView/index.ts (1)

370-377: The handlePermissionCheck handler is intentionally narrow in scope and serves a different purpose than handlePermissionRequest. The setPermissionCheckHandler responds only to navigator.permissions.query() calls, while setPermissionRequestHandler handles actual permission requests. Since permissions.query() is not used in the codebase, and screen sharing uses Jitsi's legacy chromeMediaSource constraints rather than the standard display-capture permission, the current implementation correctly addresses its targeted purpose: preventing the microphone indicator from appearing on startup. Aligning these handlers is unnecessary and could broaden the scope beyond the intended fix.

			> Likely an incorrect or invalid review comment.

[jeanfbrito]

Thanks for the PR! The approach is correct — setPermissionCheckHandler is the right way to intercept navigator.permissions.query() and prevent the OS-level getMediaAccessStatus() call that triggers the Windows mic indicator.

However, the handler should mirror the permissions already granted by setPermissionRequestHandler in the same file. Currently, the request handler grants: media, geolocation, notifications, midiSysex, pointerLock, and fullscreen. But this check handler only returns true for media and denies everything else.

This could cause inconsistencies — for example, if the webapp calls navigator.permissions.query({ name: 'notifications' }), the check handler would return denied even though the request handler would grant it.

The fix would be:

const ALLOWED_PERMISSIONS = [
  'media',
  'geolocation',
  'notifications',
  'midiSysex',
  'pointerLock',
  'fullscreen',
] as const;

const handlePermissionCheck: Parameters<
  Session['setPermissionCheckHandler']
>[0] = (_webContents, permission) => {
  return ALLOWED_PERMISSIONS.includes(permission as any);
};

Also, this PR was based on an older version of the codebase and will likely have merge conflicts with the current master branch. Could you rebase onto the latest master?