Skip to content

fix: detect screen picker type reliably inside sandboxes (#3308) - #3386

Merged
jeanfbrito merged 1 commit into
masterfrom
fix/3308-detect-picker-type-sandbox
Jul 1, 2026
Merged

fix: detect screen picker type reliably inside sandboxes (#3308)#3386
jeanfbrito merged 1 commit into
masterfrom
fix/3308-detect-picker-type-sandbox

Conversation

@jeanfbrito

@jeanfbrito jeanfbrito commented Jul 1, 2026

Copy link
Copy Markdown
Member

Problem

Fixes #3308 — the screen-share picker dialog ("Rocket.Chat wants to share your screen. Choose what you'd like to share.") opens automatically on every app launch on Linux, before any user action. Reported on Flatpak/Flathub, reproducible on 4.14.0, 4.14.1 and 4.15.0.

Root cause

detectPickerType() chooses between the XDG-portal picker and the internal cached-thumbnail picker by reading XDG_SESSION_TYPE / XDG_CURRENT_DESKTOP. Flatpak and Snap strip those variables from the sandbox environment, so on a Wayland host the function fell through to 'internal'.

The InternalPickerProvider sets requiresCacheWarming = true, which warms the desktopCapturer cache at WEBVIEW_READY via desktopCapturer.getSources(). Because we force --enable-features=WebRTCPipeWireCapturer on all Linux, getSources() on Wayland routes through the XDG portal and pops the picker dialog — at every launch, with no user involvement.

This is a picker detection gap, not a webapp probe: the Rocket.Chat web client does not call getDisplayMedia() at load (verified against the webapp call graph — the only getDisplayMedia is the user-gesture-gated VoIP path).

Fix

Flip the Linux default to 'portal' and only recover 'internal' when a silent-capable environment can be positively confirmed:

  1. ROCKETCHAT_INTERNAL_SCREEN_PICKER=1internal (escape hatch for legacy X11-without-portal setups).
  2. Wayland detected → portal. Detection no longer trusts XDG_SESSION_TYPE alone: it also accepts a real WAYLAND_DISPLAY socket on disk (fs.statSync().isSocket()), mirroring the ozone-platform decision in src/app/main/app.ts. Sandbox-safe.
  3. XDG_SESSION_TYPE=x11internal (pure X11 enumerates silently; the internal picker with cached thumbnails is preferable there).
  4. Ambiguous / stripped env (sandbox) → portal — the core fix. Worst case the picker appears on user demand (correct) instead of at launch.

Non-Linux (macOS / Windows) is unchanged → internal.

Complements the existing requiresCacheWarming guard in serverViewScreenSharing.ts: with portal correctly detected, the launch-time getSources() prewarm never runs.

Testing

  • New createScreenPicker.spec.ts — 7 cases: sandbox (no env) → portal, Wayland env → portal, Flatpak-Wayland socket → portal, X11 → internal, escape hatch → internal, macOS → internal, Windows → internal.
  • npx tsc --noEmit clean · eslint clean · jest (createScreenPicker + serverViewScreenSharing) 10/10 pass.

Impact

GitNexus impact analysis: LOW risk. Only direct caller is createScreenPicker; return type unchanged, both consumers (videoCallWindow/ipc.ts main, video-call-window.ts renderer) already switch on 'portal' | 'internal'.

Note

The Flathub package (chat.rocket.RocketChat) is community-maintained and separate from this repo's official builds; this fix flows to the AppImage/deb/rpm/snap/flatpak artifacts produced here and downstream once released.

Summary by CodeRabbit

  • New Features

    • Improved screen picker selection on Linux, making the app choose the most appropriate picker more reliably across desktop and sandboxed environments.
  • Bug Fixes

    • Reduced unexpected launch dialogs by defaulting to the portal-based picker in ambiguous Linux sessions.
    • Kept screen picker behavior unchanged on macOS and Windows.

detectPickerType() read XDG_SESSION_TYPE / XDG_CURRENT_DESKTOP to choose
between the XDG-portal picker and the internal cached-thumbnail picker.
Flatpak and Snap strip those env vars from the sandbox, so on a Wayland
host the function fell through to 'internal', whose provider sets
requiresCacheWarming = true. That warms the desktopCapturer cache at
WEBVIEW_READY via desktopCapturer.getSources(); on Wayland (we force
--enable-features=WebRTCPipeWireCapturer on all Linux) getSources()
routes through the XDG portal and pops the screen-share picker dialog on
every launch.

Flip the Linux default to 'portal' and only recover 'internal' when we
can positively confirm a silent-capable environment: a Wayland session
is detected via XDG_SESSION_TYPE or a real WAYLAND_DISPLAY socket on disk
(mirroring the ozone-platform decision in app.ts), pure X11 is detected
via XDG_SESSION_TYPE=x11, and ROCKETCHAT_INTERNAL_SCREEN_PICKER=1 forces
the internal picker. Ambiguous/stripped env now defaults to portal, so
no picker dialog fires at launch. Add createScreenPicker.spec.ts covering
sandbox, Wayland (env + socket), X11, escape hatch, and macOS/Windows.
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 30bcde30-9396-4cb0-aef2-43d2cbcf1d37

📥 Commits

Reviewing files that changed from the base of the PR and between 99a330d and a50e7f5.

📒 Files selected for processing (2)
  • src/screenSharing/screenPicker/createScreenPicker.spec.ts
  • src/screenSharing/screenPicker/createScreenPicker.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: check (ubuntu-latest)
  • GitHub Check: check (macos-latest)
  • GitHub Check: check (windows-latest)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.ts: Use TypeScript for all new code unless explicitly told otherwise
Use optional chaining with fallbacks for platform-specific APIs instead of mocking when possible. Example: const uid = process.getuid?.() ?? 1000;

Files:

  • src/screenSharing/screenPicker/createScreenPicker.spec.ts
  • src/screenSharing/screenPicker/createScreenPicker.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,ts}: MANDATORY: Use Fuselage components for all UI work. Only create custom components when Fuselage doesn't provide what's needed
Import UI components from @rocket.chat/fuselage and check Theme.d.ts for valid color tokens
Use React functional components with hooks
Use PascalCase for component file names

Files:

  • src/screenSharing/screenPicker/createScreenPicker.spec.ts
  • src/screenSharing/screenPicker/createScreenPicker.ts
**/*.spec.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use *.spec.ts file naming for Renderer process tests

Files:

  • src/screenSharing/screenPicker/createScreenPicker.spec.ts
**/*.{spec.ts,main.spec.ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Only mock platform-specific APIs when defensive coding isn't possible. Linux-only APIs requiring mocks: process.getuid(), process.getgid(), process.geteuid(), process.getegid()

Files:

  • src/screenSharing/screenPicker/createScreenPicker.spec.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Redux actions must follow FSA (Flux Standard Action) pattern
Avoid unnecessary comments — write self-documenting code through clear naming
Always verify libraries by checking official docs and .d.ts files in node_modules/. Never assume props, tokens, or APIs work without verification
Avoid subjective descriptors ('smart', 'excellent', 'dumb') in documentation and comments
Use measurable descriptions in code documentation: 'reduced memory usage', 'improved by X%' instead of subjective claims
NEVER invent metrics — don't include estimated time spent or speculated user counts. Only include numbers from actual logs, error messages, or documented sources

Files:

  • src/screenSharing/screenPicker/createScreenPicker.spec.ts
  • src/screenSharing/screenPicker/createScreenPicker.ts
🔇 Additional comments (3)
src/screenSharing/screenPicker/createScreenPicker.ts (2)

1-29: LGTM!

Also applies to: 31-61, 66-72


62-63: 🔒 Security & Privacy

Verify the unknown-Linux portal fallback cannot auto-select a source.

Line 62 routes stripped/ambiguous Linux environments to PortalPickerProvider; that provider treats any non-empty desktopCapturer.getSources() result as the selected portal source. Please verify Electron always mediates this path through a user picker in the targeted ambiguous environments, or guard against multi-source silent enumeration before relying on this fallback.

src/screenSharing/screenPicker/createScreenPicker.spec.ts (1)

1-107: LGTM!


Walkthrough

The screen picker's Wayland detection logic is rewritten to verify Wayland sessions by checking for the existence of a Wayland socket file via fs.statSync(...).isSocket(), replacing the prior XDG_CURRENT_DESKTOP regex-based approach. Ambiguous/sandboxed cases now default to 'portal'. New Jest tests cover the updated behavior.

Changes

Screen picker Wayland detection fix

Layer / File(s) Summary
Wayland session detection and selection logic
src/screenSharing/screenPicker/createScreenPicker.ts
Adds isWaylandSession() to check the Wayland socket file via fs.statSync().isSocket(); rewrites detectPickerType() so Linux prioritizes the ROCKETCHAT_INTERNAL_SCREEN_PICKER override, then Wayland socket detection ('portal'), then explicit XDG_SESSION_TYPE === 'x11' ('internal'), defaulting ambiguous/sandbox cases to 'portal'; non-Linux platforms remain 'internal'.
Picker type detection tests
src/screenSharing/screenPicker/createScreenPicker.spec.ts
Adds a new spec mocking portal/internal providers and fs.statSync, managing XDG_*/RocketChat env vars and process.platform per test, verifying 'portal' and 'internal' outcomes across sandbox, Wayland, x11, override, and non-Linux platform scenarios.

Estimated code review effort: 3 (Moderate) | ~20 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 is concise and accurately summarizes the Linux sandbox screen-picker detection fix.
Linked Issues check ✅ Passed The change addresses #3308 by preventing sandbox/Wayland Linux launches from selecting the internal picker on startup.
Out of Scope Changes check ✅ Passed The PR stays focused on screen-picker selection logic and its tests, with no obvious unrelated changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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

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

@jeanfbrito
jeanfbrito merged commit 3cacd84 into master Jul 1, 2026
10 checks passed
@jeanfbrito
jeanfbrito deleted the fix/3308-detect-picker-type-sandbox branch July 1, 2026 19:46
@jeanfbrito jeanfbrito mentioned this pull request Jul 1, 2026
2 tasks
jeanfbrito added a commit that referenced this pull request Jul 9, 2026
… rule (#3402)

* docs: post-mortems for #3308 screen-picker triggers + capture-testing rule

Two-phase investigation of the launch-time screen-share picker on Linux
Wayland (#3308, fixed by #3386 and #3400). Adds the phase-2 post-mortem
covering the startup-enumeration trigger, the Electron 40 vs 42
manifestation split, and the verification protocol; commits the phase-1
sandbox-detection post-mortem that previously lived untracked.

CLAUDE.md gains the extracted testing rule: Chromium capture/portal
behavior requires hardware GL to validate — software-rendered VMs mask
it; assert on dbus ScreenCast requests, not dialog visibility.

* docs: refresh GitNexus section in CLAUDE.md

Tool-generated section update: current index stats, short tool names
(impact/query/context/rename), detect_changes compare syntax, and the
run.cjs analyze entrypoint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Screen share picker opens on every app launch (regression since 4.14.0, likely PR #3266)

1 participant