feat(webview): in-page screen-share picker for getDisplayMedia (#713)#809
Merged
graycyrus merged 12 commits intotinyhumansai:mainfrom Apr 24, 2026
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds CEF/macOS screen-share support: new Tauri Changes
Sequence DiagramsequenceDiagram
actor User
participant WebView as WebView/JS
participant Overlay as Overlay Picker
participant Tauri as Tauri Cmds (Rust)
participant CG as macOS CoreGraphics
participant Browser as Browser getUserMedia
User->>WebView: navigator.mediaDevices.getDisplayMedia()
WebView->>Tauri: invoke screen_share_begin_session (has_user_activation)
Tauri->>Tauri: enforce rate limits, create token, enumerate allowed IDs
Tauri->>CG: enumerate displays & windows (macOS)
CG-->>Tauri: source metadata
Tauri->>Tauri: capture/encode thumbnails (PNG → base64)
Tauri-->>WebView: BeginSessionResult { token, sources }
WebView->>Overlay: render picker with sources
User->>Overlay: select source
Overlay-->>WebView: chosen source id
WebView->>Tauri: (optional) screen_share_thumbnail with token,id
Tauri-->>WebView: thumbnail (base64) or error
WebView->>Browser: getUserMedia({ chromeMediaSource: 'desktop', chromeMediaSourceId })
Browser-->>WebView: MediaStream
WebView->>WebView: set contentHint & override track.getSettings()
WebView->>Tauri: screen_share_finalize_session(token)
Tauri->>Tauri: remove session (no-op if expired)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/src-tauri/src/screen_capture/mod.rs (1)
61-94: Add command-level debug logs for the new screen-share IPCs.Right now failures are mostly silent unless CoreGraphics returns a low-level error. When the picker comes up empty or a thumbnail never arrives, there is no log with the command path, source id, or returned count to correlate from the webview side. Please add
debuglogs on entry/exit with a stable[screen-share]prefix.As per coding guidelines, "
**/*.rs: Uselog/tracingatdebug/tracelevels. Default to verbose diagnostics on new/changed flows with stable grep-friendly prefixes ([domain],[rpc],[ui-flow]) and correlation fields (request IDs, method names, entity IDs)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/screen_capture/mod.rs` around lines 61 - 94, Add debug-entry/exit tracing to the new screen-share IPC commands: in screen_share_list_sources() log a debug line on entry like "[screen-share] screen_share_list_sources request" and on successful return include the returned Vec length (and on Err include the error) so callers can correlate picker results; in screen_share_thumbnail(args: ThumbnailArgs) log on entry including the args.id ("[screen-share] screen_share_thumbnail request id={id}") and on success return include a debug log with id and that a thumbnail was returned (or on Err log id plus the error). Use the existing macos::enumerate and macos::thumbnail_for_id call sites to attach the logs, follow project convention using the log/tracing debug level and the stable "[screen-share]" prefix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src-tauri/permissions/allow-webview-recipe.toml`:
- Around line 3-6: Remove the raw screen-source commands from the allow list so
third-party pages cannot directly invoke them: in the allow-webview-recipe
permission block, delete "screen_share_list_sources" and
"screen_share_thumbnail" from permission.commands.allow, leaving only safe
commands like "webview_recipe_event"; instead implement a host-mediated flow
where the webview calls a new safe command (e.g., request_screen_share_session)
that returns a one-shot session nonce, and the host (Rust) uses that nonce to
run the picker and deliver results back to the webview via a controlled channel;
update handlers for screen_share_list_sources/screen_share_thumbnail to only
accept requests tied to a valid server-generated nonce/session and reject direct
invocations from the page.
In `@app/src-tauri/src/webview_accounts/runtime.js`:
- Around line 656-678: The helper window.__ohScreenShareTest is a global
screen-capture bypass and must not be exposed in production; either remove its
definition entirely or wrap it so it only attaches in non-production/dev builds
(e.g., check an explicit dev flag like process.env.NODE_ENV !== 'production' or
a runtime isDev boolean) and also remove the hardcoded default sourceId
('screen:1:0') so it never attempts a default capture; update the runtime.js
code that defines window.__ohScreenShareTest to only create the function when
running in a dev-only context and ensure no global reference remains in
production.
- Around line 453-485: render currently fires
rawInvoke('screen_share_thumbnail', ...) on every re-render while
src.thumbnailPngBase64 is unset; to deduplicate, add and check an in-flight
promise/flag on the source (e.g. src._thumbnailRequest) before calling
rawInvoke: if src.thumbnailPngBase64 exists return; if src._thumbnailRequest
exists attach a .then handler to it to update the live node; otherwise set
src._thumbnailRequest = rawInvoke(...).then(b64 => { if (typeof b64 ===
'string') { src.thumbnailPngBase64 = b64; /* update liveThumb same as current
code */ } return b64 }).catch(()=>{}).finally(()=>{ delete src._thumbnailRequest
}); update all places that currently call rawInvoke('screen_share_thumbnail',
...) to use this pattern so each source only issues one in-flight request.
- Around line 279-283: The DOM-not-ready branch currently calls
resolveOuter(null) causing an artificial AbortError; instead, wait for
DOMContentLoaded and then re-check host() and proceed with the normal picker
flow (i.e., call the same code path that would run when host() is available) or,
if host() is still falsy after the event, fall back to the original
getDisplayMedia implementation and resolve/reject accordingly. Update the block
that adds the DOMContentLoaded listener (referencing host() and resolveOuter) so
the handler re-evaluates host() and either invokes the host-based picker flow or
delegates to the original getDisplayMedia behavior rather than resolving null
immediately.
---
Nitpick comments:
In `@app/src-tauri/src/screen_capture/mod.rs`:
- Around line 61-94: Add debug-entry/exit tracing to the new screen-share IPC
commands: in screen_share_list_sources() log a debug line on entry like
"[screen-share] screen_share_list_sources request" and on successful return
include the returned Vec length (and on Err include the error) so callers can
correlate picker results; in screen_share_thumbnail(args: ThumbnailArgs) log on
entry including the args.id ("[screen-share] screen_share_thumbnail request
id={id}") and on success return include a debug log with id and that a thumbnail
was returned (or on Err log id plus the error). Use the existing
macos::enumerate and macos::thumbnail_for_id call sites to attach the logs,
follow project convention using the log/tracing debug level and the stable
"[screen-share]" prefix.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46d70bc1-99fb-488f-876e-02349da0da3a
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/src-tauri/Cargo.tomlapp/src-tauri/permissions/allow-core-process.tomlapp/src-tauri/permissions/allow-webview-recipe.tomlapp/src-tauri/src/lib.rsapp/src-tauri/src/screen_capture/mod.rsapp/src-tauri/src/webview_accounts/runtime.js
Comment on lines
+3
to
+6
| description = "Allow injected per-provider recipe code (running inside the third-party site's origin) to invoke the recipe ingest command back to Rust. Also includes screen_share_list_sources so the in-page getDisplayMedia shim (#713) can enumerate real screens and windows to build its picker UI inside the child webview's DOM." | ||
|
|
||
| [permission.commands] | ||
| allow = ["webview_recipe_event"] | ||
| allow = ["webview_recipe_event", "screen_share_list_sources", "screen_share_thumbnail"] |
Contributor
There was a problem hiding this comment.
Do not expose raw screen-source commands to the third-party page context.
This capability runs inside the embedded site's origin, so the site itself can call window.__TAURI_INTERNALS__.invoke('screen_share_list_sources'|'screen_share_thumbnail'). That exposes cross-app window titles and live thumbnails before the user has chosen anything in the picker, which is a privacy/security blocker. Keep these commands out of allow-webview-recipe and route picker data through a host-owned, one-shot session/nonce instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/permissions/allow-webview-recipe.toml` around lines 3 - 6,
Remove the raw screen-source commands from the allow list so third-party pages
cannot directly invoke them: in the allow-webview-recipe permission block,
delete "screen_share_list_sources" and "screen_share_thumbnail" from
permission.commands.allow, leaving only safe commands like
"webview_recipe_event"; instead implement a host-mediated flow where the webview
calls a new safe command (e.g., request_screen_share_session) that returns a
one-shot session nonce, and the host (Rust) uses that nonce to run the picker
and deliver results back to the webview via a controlled channel; update
handlers for screen_share_list_sources/screen_share_thumbnail to only accept
requests tied to a valid server-generated nonce/session and reject direct
invocations from the page.
Contributor
Author
There was a problem hiding this comment.
Tracked as follow-up in #812 — host-driven session/nonce flow requires adjusting the vendored tauri-cef media-access callback so it can be held while the shim renders the picker, which is a bigger change than landing the immediate two-critical/two-major fixes. Leaving this PR's allowlist as-is but ack'd for the next round.
Contributor
There was a problem hiding this comment.
@oxoxDev, understood — thanks for the context. The tauri-cef media-access callback constraint is a reasonable blocker for a proper nonce/session flow. I'll track this against #812 and re-raise it there. Happy to help design the host-mediated IPC shape when that work starts.
oxoxDev
added a commit
to oxoxDev/openhuman
that referenced
this pull request
Apr 22, 2026
) Picks up the 2 critical + 2 major findings on tinyhumansai#809 that do not require a host/tauri-cef architecture change: - Delete `window.__ohScreenShareTest` entirely. The PoC helper was a page-global screen-capture primitive (default `screen:1:0`, no picker) that any third-party script in the embedded site could have invoked to capture the main display silently. We have the live in-page picker now; the helper has no production value. - Retry `showInPagePicker` after `DOMContentLoaded` when `document.body` isn't ready yet, instead of resolving `null` and letting the shim surface an `AbortError` for a picker that was never actually shown. - Deduplicate in-flight `screen_share_thumbnail` IPCs per source. The picker's `render()` runs again on every selection change and tab switch; without a cache, every un-cached source would re-issue its thumbnail capture on each pass. Cache the in-flight promise on the source object and reuse it, which also lets all pending render passes settle when the first one completes. The third critical finding (commands still reachable from the recipe origin via direct `invoke()`) needs a host-driven nonce/session flow and lands in a follow-up PR — see issue tracker. Refs coderabbit review on tinyhumansai#809. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src-tauri/src/webview_accounts/runtime.js (1)
531-536: Consider validatingselectedIdbefore the render loop.The current flow renders buttons, then checks if
selectedIdis valid, then manually adds theselectedclass tofirstChild. This works but requires the post-render fixup. Moving the validation before theforEachloop would make the code flow clearer.♻️ Suggested simplification
function render() { while (gridEl.firstChild) gridEl.removeChild(gridEl.firstChild); const filtered = sources.filter(function (s) { return s.kind === activeTab; }); if (filtered.length === 0) { // ... empty state handling return; } + // Validate selectedId before rendering so buttons get correct 'selected' class + if (!selectedId || !filtered.some(function (s) { return s.id === selectedId; })) { + selectedId = filtered[0].id; + } filtered.forEach(function (src) { const btn = document.createElement('button'); btn.className = 'srcbtn' + (selectedId === src.id ? ' selected' : ''); // ... }); - if (!selectedId || !filtered.some(function (s) { return s.id === selectedId; })) { - selectedId = filtered[0].id; - gridEl.firstChild && gridEl.firstChild.classList.add('selected'); - } shareBtn.disabled = !selectedId; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/webview_accounts/runtime.js` around lines 531 - 536, Validate and normalize selectedId before the render loop: check if selectedId is falsy or not present in filtered and if so set selectedId = filtered[0].id, then proceed into the forEach that creates children so you can add the 'selected' class to the correct element during element creation (using gridEl child creation logic) instead of adding gridEl.firstChild.classList.add('selected') after rendering; also ensure shareBtn.disabled is computed from the validated selectedId after the loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src-tauri/src/webview_accounts/runtime.js`:
- Around line 531-536: Validate and normalize selectedId before the render loop:
check if selectedId is falsy or not present in filtered and if so set selectedId
= filtered[0].id, then proceed into the forEach that creates children so you can
add the 'selected' class to the correct element during element creation (using
gridEl child creation logic) instead of adding
gridEl.firstChild.classList.add('selected') after rendering; also ensure
shareBtn.disabled is computed from the validated selectedId after the loop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a3404d6d-693c-4721-8c56-96a8988cd8e6
📒 Files selected for processing (2)
app/src-tauri/src/screen_capture/mod.rsapp/src-tauri/src/webview_accounts/runtime.js
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src-tauri/src/screen_capture/mod.rs
Contributor
|
Hey @oxoxDev — this PR has merge conflicts with |
…inyhumansai#713) New `screen_capture` module exposes two Tauri commands that back the in-page getDisplayMedia picker shim: - `screen_share_list_sources` enumerates real screens (CGGetActiveDisplayList) and on-screen windows (CGWindowListCopyWindowInfo), tagging each with a Chromium-compatible DesktopMediaID string (`screen:<CGDirectDisplayID>:0` / `window:<CGWindowID>:0`). No thumbnails are captured during enumeration so the picker opens instantly. - `screen_share_thumbnail` captures a single source's live thumbnail on demand via CGDisplayCreateImage / CGWindowListCreateImage, encodes it to PNG through ImageIO, and returns base64. The picker shim fires these in parallel so thumbnails fade in as they arrive. Uses raw CoreGraphics + CoreFoundation + ImageIO FFI so we don't pull in the full `core-graphics` crate (~50 extra transitive deps). Adds the existing-workspace `base64` crate for PNG serialization. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wire `screen_share_list_sources` and `screen_share_thumbnail` into the Tauri invoke_handler so the getDisplayMedia shim injected into child webviews (Meet, Slack, Discord, Zoom) can reach them over IPC. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ebviews (tinyhumansai#713) Add `screen_share_list_sources` and `screen_share_thumbnail` to both permission scopes: - `allow-core-process` so the host window can call them if a future surface ever needs enumeration host-side. - `allow-webview-recipe` so the per-provider recipe runtime (which now includes the in-page getDisplayMedia picker) can enumerate and fetch thumbnails directly from the third-party origin. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…umansai#713) Child webviews (Meet, Slack Huddles, Discord, Zoom) run under CEF Alloy, which does not link Chromium's DesktopMediaPicker. Without an interceptor the permission handler auto-grants `DESKTOP_VIDEO_CAPTURE` and Chromium silently picks the primary display — the user never sees a picker and their whole screen streams. This shim, injected via `WebviewBuilder.initialization_script`: - Overrides `navigator.mediaDevices.getDisplayMedia` on the MediaDevices prototype (plain assignment is non-writable in modern Chromium, so we `Object.defineProperty` on the prototype). - Enumerates real sources via the new `screen_share_list_sources` Tauri command, then renders a picker overlay directly into the host page's DOM (no main-window React modal) so it works uniformly for every provider without per-provider host-side glue and without native-view stacking problems. Picker DOM is built imperatively to satisfy Meet's Trusted Types CSP (innerHTML would throw). - Lazy-fetches thumbnails in parallel through `screen_share_thumbnail` so the picker opens instantly and images fade in. - On Share, calls `getUserMedia` with a hand-crafted `chromeMediaSource: 'desktop' + chromeMediaSourceId` constraint. Our CEF permission callback grants `DESKTOP_VIDEO_CAPTURE` bits, so Chromium honours the ID and opens a real capture device. - Patches the returned track's `getSettings()` to expose `displaySurface: 'monitor' | 'window'` and `contentHint: 'detail'`, because Meet refuses streams that lack the display-media metadata real getDisplayMedia would populate. - Falls back to video-only if the page asked for audio but audio+video getUserMedia rejects (stock macOS has no loopback audio driver). - Throws `AbortError` on picker cancel so Meet silently dismisses instead of showing "Meet needs permission to screen share". - Shims `navigator.permissions.query` to report `display-capture` as `granted`; CEF Alloy's Permissions API defaults to `prompt`/`denied` and pages that consult it otherwise refuse to call getDisplayMedia at all. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Extract the `screen:<id>:0` / `window:<id>:0` ID parser out of the macOS-only thumbnail helper into a platform-agnostic `parse_source_id` and add unit tests for: - valid screen and window IDs - trailing segments beyond the canonical `:0` - unknown kind prefixes (tab, browser, empty) - missing or non-numeric numeric segment - u32 overflow and negative numbers - enumerator → parser round-trip shape The parser is where bad shim input would silently degrade to "thumbnail unavailable" in production, so pinning its contract keeps the shim/host boundary honest even though the surrounding macOS capture layer itself can only be exercised in live-app testing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…#713) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
) Picks up the 2 critical + 2 major findings on tinyhumansai#809 that do not require a host/tauri-cef architecture change: - Delete `window.__ohScreenShareTest` entirely. The PoC helper was a page-global screen-capture primitive (default `screen:1:0`, no picker) that any third-party script in the embedded site could have invoked to capture the main display silently. We have the live in-page picker now; the helper has no production value. - Retry `showInPagePicker` after `DOMContentLoaded` when `document.body` isn't ready yet, instead of resolving `null` and letting the shim surface an `AbortError` for a picker that was never actually shown. - Deduplicate in-flight `screen_share_thumbnail` IPCs per source. The picker's `render()` runs again on every selection change and tab switch; without a cache, every un-cached source would re-issue its thumbnail capture on each pass. Cache the in-flight promise on the source object and reuse it, which also lets all pending render passes settle when the first one completes. The third critical finding (commands still reachable from the recipe origin via direct `invoke()`) needs a host-driven nonce/session flow and lands in a follow-up PR — see issue tracker. Refs coderabbit review on tinyhumansai#809. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
yes bro doing it right away |
oxoxDev
force-pushed
the
feat/713-screen-share-picker
branch
from
ee527ab to
4c9274f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src-tauri/src/screen_capture/mod.rs (3)
444-451: Incomplete bounds filtering — documented as intentional MVP scope.The bounds check code structure exists but doesn't implement the actual size filter. The comment explains this is intentional for MVP. Consider adding a TODO or follow-up issue reference for completing this filter to exclude tooltips/tiny panels.
📝 Optional: Add TODO for future bounds filtering
// kCGWindowBounds is actually a CFDictionary with Width/Height // keys. Cheap filter: if the dict has a "Width" key and it's // < 50, skip. Implementing full parse isn't worth it for the // MVP; Chromium renders a scrollable picker anyway. + // TODO(`#713`): Implement bounds filtering to exclude tooltips/tiny panels let _ = bounds_dict;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/screen_capture/mod.rs` around lines 444 - 451, The bounds check block around CFDictionaryGetValue(dict, key_bounds) currently creates bounds_dict but does not perform the documented Width/Height filtering; either implement the simple size filter (read the "Width" key from bounds_dict and skip when < 50) or at minimum add a clear TODO with a tracking issue reference so future work isn't lost; update the block around CFDictionaryGetValue, dict, key_bounds and bounds_dict to either perform the Width check or include a TODO comment referencing the issue number and explaining that tiny windows (e.g., tooltips with Width < 50) should be excluded.
434-437: Window ID type mismatch is benign but could be cleaner.
cfnumber_to_u64returnsu64, butCGWindowIDisu32. The string formatting works correctly, but for type safety you could cast tou32when constructing the ID string to match the ChromiumDesktopMediaIDexpectation.♻️ Optional: Explicit u32 cast for clarity
let window_id = match cfnumber_to_u64(number_cf) { - Some(v) => v, + Some(v) => v as u32, None => continue, };Also applies to: 467-468
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/screen_capture/mod.rs` around lines 434 - 437, The window ID value returned by cfnumber_to_u64 is a u64 but the Chromium DesktopMediaID expects a CGWindowID (u32); in the code paths that use window_id (the variable set from cfnumber_to_u64 and later when building the ID string), cast the u64 to u32 (e.g., let window_id_u32 = window_id as u32) before formatting the DesktopMediaID so types match and intent is explicit; apply the same explicit cast in the other occurrence noted around the cfnumber_to_u64 usage at the second location (lines near the other cfnumber_to_u64 call).
61-71: Consider adding debug-level logging for command entry.Per coding guidelines, new/changed flows should have verbose diagnostics. Adding a
log::debug!("[screen-share] list_sources called")at entry would aid debugging without impacting performance.🔧 Optional: Add entry logging
#[tauri::command] pub fn screen_share_list_sources() -> Result<Vec<ScreenSource>, String> { + log::debug!("[screen-share] list_sources invoked"); #[cfg(target_os = "macos")] { macos::enumerate().map_err(|e| format!("enumerate failed: {e}"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/screen_capture/mod.rs` around lines 61 - 71, Add a debug-level entry log to the screen_share_list_sources command so callers are recorded; specifically, at the top of the pub fn screen_share_list_sources() add a log::debug!("[screen-share] list_sources called") (or use debug! if the log macro is already in scope). Keep the log call unconditional and before the cfg blocks so it runs on all platforms; ensure the log crate/macros are available in this module if not already.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src-tauri/src/screen_capture/mod.rs`:
- Around line 61-71: Add documentation entries for the two Tauri IPC commands
screen_share_list_sources and screen_share_thumbnail to the project's Tauri
commands documentation (02-commands.md): create a new "Screen Capture" section
(or add to the appropriate section), describe each command's purpose,
input/output types and return values, and clearly note they are currently
macOS-only; ensure the command names exactly match the exported functions
(screen_share_list_sources, screen_share_thumbnail) so the docs align with the
registered IPC handlers.
---
Nitpick comments:
In `@app/src-tauri/src/screen_capture/mod.rs`:
- Around line 444-451: The bounds check block around CFDictionaryGetValue(dict,
key_bounds) currently creates bounds_dict but does not perform the documented
Width/Height filtering; either implement the simple size filter (read the
"Width" key from bounds_dict and skip when < 50) or at minimum add a clear TODO
with a tracking issue reference so future work isn't lost; update the block
around CFDictionaryGetValue, dict, key_bounds and bounds_dict to either perform
the Width check or include a TODO comment referencing the issue number and
explaining that tiny windows (e.g., tooltips with Width < 50) should be
excluded.
- Around line 434-437: The window ID value returned by cfnumber_to_u64 is a u64
but the Chromium DesktopMediaID expects a CGWindowID (u32); in the code paths
that use window_id (the variable set from cfnumber_to_u64 and later when
building the ID string), cast the u64 to u32 (e.g., let window_id_u32 =
window_id as u32) before formatting the DesktopMediaID so types match and intent
is explicit; apply the same explicit cast in the other occurrence noted around
the cfnumber_to_u64 usage at the second location (lines near the other
cfnumber_to_u64 call).
- Around line 61-71: Add a debug-level entry log to the
screen_share_list_sources command so callers are recorded; specifically, at the
top of the pub fn screen_share_list_sources() add a log::debug!("[screen-share]
list_sources called") (or use debug! if the log macro is already in scope). Keep
the log call unconditional and before the cfg blocks so it runs on all
platforms; ensure the log crate/macros are available in this module if not
already.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 43892bbf-4d2d-4eef-80f2-1f682218722d
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/src-tauri/Cargo.tomlapp/src-tauri/permissions/allow-core-process.tomlapp/src-tauri/permissions/allow-webview-recipe.tomlapp/src-tauri/src/lib.rsapp/src-tauri/src/screen_capture/mod.rsapp/src-tauri/src/webview_accounts/runtime.js
✅ Files skipped from review due to trivial changes (3)
- app/src-tauri/Cargo.toml
- app/src-tauri/permissions/allow-core-process.toml
- app/src-tauri/permissions/allow-webview-recipe.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src-tauri/src/webview_accounts/runtime.js
Comment on lines
+61
to
+71
| #[tauri::command] | ||
| pub fn screen_share_list_sources() -> Result<Vec<ScreenSource>, String> { | ||
| #[cfg(target_os = "macos")] | ||
| { | ||
| macos::enumerate().map_err(|e| format!("enumerate failed: {e}")) | ||
| } | ||
| #[cfg(not(target_os = "macos"))] | ||
| { | ||
| Err("screen-share picker not implemented for this platform yet".to_string()) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the new commands are documented
rg -i 'screen_share' docs/src-tauri/02-commands.md 2>/dev/null || echo "File not found or commands not documented"Repository: tinyhumansai/openhuman
Length of output: 107
🏁 Script executed:
# Check if docs/src-tauri/02-commands.md exists and what's in it
fd -type f 'commands.md' docs/ 2>/dev/null || echo "Not in docs/"
# Also search more broadly
fd -type f 'commands.md' . 2>/dev/null | head -20Repository: tinyhumansai/openhuman
Length of output: 78
🏁 Script executed:
# Check the exact commands at the specified lines in the file
sed -n '61,71p' app/src-tauri/src/screen_capture/mod.rs
echo "---"
sed -n '107,118p' app/src-tauri/src/screen_capture/mod.rsRepository: tinyhumansai/openhuman
Length of output: 797
🏁 Script executed:
# Check if there's any documentation structure for Tauri commands elsewhere
fd -type f '*.md' docs/ | head -20Repository: tinyhumansai/openhuman
Length of output: 236
🏁 Script executed:
# Let's properly search for markdown files in docs
find docs -name "*.md" 2>/dev/null | head -20Repository: tinyhumansai/openhuman
Length of output: 535
🏁 Script executed:
# Check if docs directory exists at all
ls -la docs/ 2>/dev/null || echo "docs/ directory not found"
# Also check from root
ls -la . | grep -i docRepository: tinyhumansai/openhuman
Length of output: 2472
🏁 Script executed:
# Search for any reference to "screen_share" in the entire repo to understand documentation patterns
rg 'screen_share' --type md 2>/dev/null | head -20Repository: tinyhumansai/openhuman
Length of output: 48
🏁 Script executed:
# Check what's in docs/src-tauri directory
ls -la docs/src-tauri/Repository: tinyhumansai/openhuman
Length of output: 432
🏁 Script executed:
# Now let's check the content of 02-commands.md if it exists
cat docs/src-tauri/02-commands.md 2>/dev/null || echo "File does not exist"Repository: tinyhumansai/openhuman
Length of output: 3722
🏁 Script executed:
# Let's also check the lib.rs file to see how other commands are documented (if at all)
rg '#\[tauri::command\]' app/src-tauri/src/lib.rs -A 2 | head -40Repository: tinyhumansai/openhuman
Length of output: 1365
🏁 Script executed:
# Check if screen_share commands are registered in the generate_handler macro in lib.rs
rg 'generate_handler' app/src-tauri/src/lib.rs -A 50Repository: tinyhumansai/openhuman
Length of output: 2364
🏁 Script executed:
# Also check if screen_share module is imported and exported properly
rg 'screen_share\|screen_capture' app/src-tauri/src/lib.rsRepository: tinyhumansai/openhuman
Length of output: 48
Document the screen capture commands in docs/src-tauri/02-commands.md.
The screen_share_list_sources and screen_share_thumbnail commands are registered in the Tauri IPC handler but not documented. Per coding guidelines, all Tauri IPC commands must be documented. Add these commands to the appropriate section (or create a new "Screen Capture" section) in docs/src-tauri/02-commands.md, noting that they are currently macOS-only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/screen_capture/mod.rs` around lines 61 - 71, Add
documentation entries for the two Tauri IPC commands screen_share_list_sources
and screen_share_thumbnail to the project's Tauri commands documentation
(02-commands.md): create a new "Screen Capture" section (or add to the
appropriate section), describe each command's purpose, input/output types and
return values, and clearly note they are currently macOS-only; ensure the
command names exactly match the exported functions (screen_share_list_sources,
screen_share_thumbnail) so the docs align with the registered IPC handlers.
graycyrus
requested changes
Apr 23, 2026
Contributor
There was a problem hiding this comment.
Code Review — feat(webview): in-page screen-share picker for getDisplayMedia (#713)
Walkthrough
This PR fills AC2 of #713 by giving embedded webviews (Meet, Slack Huddles, Discord, Zoom) a real screen/window picker instead of letting CEF silently auto-capture the primary display. The architecture decision — injecting the picker directly into the child webview's DOM rather than rendering a React modal in the host window — is the right call given CEF's NSView stacking constraints. The Rust half is a clean, self-contained CoreGraphics FFI module; the JavaScript half is well-defended against Trusted Types CSP, handles the getSettings metadata gap that breaks Meet, and covers the DOMContentLoaded edge case from the previous review round.
Changes
| File | Summary |
|---|---|
app/src-tauri/src/screen_capture/mod.rs |
New module: CoreGraphics FFI for display/window enumeration and thumbnail capture; two Tauri commands; parse_source_id with unit tests |
app/src-tauri/src/webview_accounts/runtime.js |
installGetDisplayMediaShim IIFE: overrides getDisplayMedia, renders in-page picker overlay, patches getSettings, shims permissions.query |
app/src-tauri/src/lib.rs |
Registers two new commands under #[cfg(feature = "cef")] |
app/src-tauri/permissions/allow-webview-recipe.toml |
Adds screen_share_list_sources + screen_share_thumbnail to recipe-webview scope |
app/src-tauri/permissions/allow-core-process.toml |
Same two commands added to core-process scope |
app/src-tauri/Cargo.toml |
Adds base64 = "0.22" |
Blockers (2)
1. screen_share_thumbnail callable by any page script without user authorization
Files: allow-webview-recipe.toml:6, screen_capture/mod.rs:108-118
screen_share_thumbnail is allowed from the allow-webview-recipe scope, which covers every account webview including those loaded on https://meet.google.com, https://slack.com, etc. window.__TAURI_INTERNALS__.invoke is accessible to the page's own JavaScript, not just OpenHuman's injected shim. Any script running in those webviews — including third-party scripts loaded by Meet or a compromised CDN — can silently call:
window.__TAURI_INTERNALS__.invoke('screen_share_thumbnail', { id: 'screen:69734208:0' })and receive a full-resolution base64 PNG of the user's display, with no picker interaction, no TCC prompt re-check, and no indication to the user. Compare with webview_recipe_event, which verifies that the calling webview's Tauri label matches the account_id in the payload.
Suggested fix — add caller verification:
#[tauri::command]
pub fn screen_share_thumbnail(
webview: tauri::Webview<impl tauri::Runtime>,
args: ThumbnailArgs,
) -> Result<String, String> {
if !webview.label().starts_with("acct_") {
log::warn!(
"[screen-share] screen_share_thumbnail rejected: caller={} is not an account webview",
webview.label()
);
return Err("unauthorized caller".to_string());
}
// ... rest of implementation
}For tighter control, a per-webview atomic flag set when screen_share_list_sources is called and cleared after the stream is obtained would prevent out-of-band capture requests entirely.
2. CGWindowID stored as u64 but parsed back as u32 — silent thumbnail failures for large IDs
File: screen_capture/mod.rs:434-468
cfnumber_to_u64 returns u64, and window_id is declared as u64. The ID is formatted with format!("window:{}:0", window_id). parse_source_id later calls parts.next()?.parse::<u32>().ok()? — so if window_id > u32::MAX, the parse fails and thumbnails silently return None. The user sees a source in the picker with a permanent grey placeholder.
CGWindowID is uint32_t so values above u32::MAX shouldn't exist in practice, but the type mismatch means a future change could silently break things.
Suggested fix:
let window_id_u64 = match cfnumber_to_u64(number_cf) {
Some(v) => v,
None => continue,
};
let window_id = match u32::try_from(window_id_u64) {
Ok(v) => v,
Err(_) => {
log::warn!("[screen-share] window_id {} overflows u32, skipping", window_id_u64);
continue;
}
};Major (3)
3. cfstr uses expect() in FFI context — UB if it ever panics
File: screen_capture/mod.rs:241
CString::new(s).expect("cfstr contains NUL") causes an unwinding panic, and panicking through a C frame is undefined behavior. The five key strings will never contain NUL, but the pattern is unsafe by construction.
Suggested fix — return Option instead:
fn cfstr(s: &str) -> Option<*const c_void> {
let c = std::ffi::CString::new(s).ok()?;
let ptr = unsafe {
CFStringCreateWithCString(std::ptr::null(), c.as_ptr(), K_CFSTRING_ENCODING_UTF8)
};
if ptr.is_null() { None } else { Some(ptr) }
}4. CGWindowListCreateImage returns 1×1 transparent image on macOS 15 Sequoia without Screen Recording grant
File: screen_capture/mod.rs:353-374
The current guard only checks CGImageGetWidth == 0. On Sequoia, it returns a valid 1×1 transparent image instead of null when TCC is not granted. Users get a valid-but-useless 1px PNG that passes through the shim's empty-string check.
Suggested fix — add minimum-dimension guard:
if CGImageGetWidth(image) < 4 || CGImageGetHeight(image) < 4 {
log::warn!(
"[screen-share] window_thumbnail_b64 id={} returned {}×{} image \
(likely TCC not granted or Sequoia privacy policy)",
window_id, CGImageGetWidth(image), CGImageGetHeight(image)
);
CGImageRelease(image);
return String::new();
}5. No debug logging on new IPC commands
File: screen_capture/mod.rs:62-118
Per project guidelines, new flows must have entry/exit logging with grep-friendly prefixes. Neither command emits any logs on entry, success, or platform-stub fallback.
Suggested additions:
pub fn screen_share_list_sources() -> Result<Vec<ScreenSource>, String> {
log::debug!("[screen-share] screen_share_list_sources invoked");
// ...
log::debug!("[screen-share] screen_share_list_sources count={}", result.len());
}
pub fn screen_share_thumbnail(args: ThumbnailArgs) -> Result<String, String> {
log::debug!("[screen-share] screen_share_thumbnail id={}", args.id);
// ...
}Refactor suggestion (1)
6. Concurrent getDisplayMedia calls produce stacked picker overlays
File: runtime.js:824-838
If getDisplayMedia is called twice before the first picker resolves, two overlays are appended to document.body. The inner settled guard prevents double-resolution of the same promise, but the second overlay is never cleaned up.
Suggested fix — in-flight guard:
let pickerInFlight = false;
const shim = async function (constraints) {
if (pickerInFlight) {
throw new DOMException('Picker already open', 'InvalidStateError');
}
pickerInFlight = true;
try { /* ... existing logic ... */ }
finally { pickerInFlight = false; }
};Nitpicks (4)
screen_capture/mod.rs:444-451— The bounds-filter block is a no-op (let _ = bounds_dict). Either remove the block or replace with a TODO comment referencing #713.docs/src-tauri/02-commands.md— New IPC commands are absent from the command catalog. One table row each with the#[cfg(feature = "cef")] / macOSqualifier is enough.runtime.js:1180—permissions.queryspoof only coversdisplay-capture. If CEF Alloy also reportscamera/microphoneasdenied, consider adding them to the spoof map.screen_capture/mod.rs:237—K_CG_WINDOW_LIST_EXCLUDE_DESKTOP_ELEMENTS = 1 << 4— add// = 16inline comment for clarity.
Verified / Looks good ✓
- CFRelease patterns are balanced across all exit paths — no memory leaks
parse_source_idcorrectly usessplitn(3, ':')— unit tests cover edge cases- DOMContentLoaded retry correctly forwards both
resolveOuterandrejectOuter - Shadow root
mode: 'closed'prevents host page from querying picker DOM getSettingsoverride has try/catch fallback for non-configurable track descriptors- Trusted Types CSP compliance (no
innerHTML, all DOM built imperatively) rawInvokegracefully handles missing__TAURI_INTERNALS__K_CFNUMBER_SINT64_TYPE = 4matches Apple's header value- Negative layer values correctly filtered by the
layer != 0check
…umansai#812) Replaces the flat `screen_share_list_sources` + `screen_share_thumbnail` command pair with a short-lived session token flow, closing the privacy surface graycyrus and CodeRabbit flagged as a blocker on tinyhumansai#809: page JS (including third-party scripts loaded by the embedded site) could previously call either command directly and exfiltrate open window titles + live thumbnails with no picker interaction and no user gesture. Session gating (tinyhumansai#812 Stage A): - `screen_share_begin_session` — opens a 30s session. Requires (1) the caller's webview label to start with `acct_` (rejects main/overlay windows), (2) a live `navigator.userActivation.isActive` (frontend check; rejects timers and async continuations), and (3) ≤10 begin attempts per account in any 60s window. Returns `{ token, sources }` in one round-trip so the picker opens with zero extra latency. - `screen_share_thumbnail` — now requires the token from a live session AND an `id` that was in that session's enumerated source list. A leaked token can only replay thumbnails for the IDs the shim already showed the user. - `screen_share_finalize_session` — explicit cleanup from the shim on Share or Cancel. No-op on unknown tokens. Not label-gated because the only effect is dropping a token the caller already possesses. A new begin_session for an account replaces any in-flight session for the same account, which also fixes graycyrus' refactor note tinyhumansai#6 (concurrent getDisplayMedia calls producing stacked overlays) at the host-state level. Review blockers / majors (graycyrus, tinyhumansai#809): - `CGWindowID` was flowing through `cfnumber_to_u64` (u64) and being formatted straight into the DesktopMediaID. `parse_source_id` accepts `u32` only, so a window with an ID above u32::MAX would have been silently dropped with a grey placeholder. Enumerator now does a checked `u32::try_from` and logs + skips on overflow. - `cfstr` was calling `CString::new().expect()`; panicking through the Apple FFI frames is UB. Returns `Option<*const c_void>` now, callers bail cleanly. - `screen_thumbnail_b64` / `window_thumbnail_b64` now guard against images smaller than 4×4 pixels. macOS 15 Sequoia returns a valid 1×1 transparent CGImage when Screen Recording TCC is not granted (instead of the pre-Sequoia null); the old zero-dimension check wasn't catching that. - All three commands emit entry/exit `[screen-share]` debug logs with grep-friendly token prefixes so the flow is traceable in dev builds. - Dropped the no-op `bounds_dict` block from window enumeration; it was a TODO placeholder that never gained a filter. - Added `= 8` / `= 16` inline hex comments to the CG window-list bitmask constants. Unit tests cover: parse_source_id (existing), token generation / URL-safe shape / uniqueness, session expiry purge, rate-limit window (11th call blocked, scoped per account). Command-level tests would need a `tauri::Webview` mock the stable API doesn't expose; the live run will exercise the gate wiring. Permission allowlists switch from listing `screen_share_list_sources` to the three session commands on both `allow-webview-recipe` and `allow-core-process`. `docs/src-tauri/02-commands.md` gains a Screen-share section describing each command. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…inyhumansai#812) Matches the new host-side screen_share_* command shapes: - At shim entry, read `navigator.userActivation.isActive`. If false, fall through to the original `getDisplayMedia` rather than open the picker. Keeps drive-by enumeration from a non-gesture context from ever reaching begin_session — matches graycyrus' "verify calling webview + gesture" gate from the tinyhumansai#809 review. - Replace `rawInvoke('screen_share_list_sources', {})` with `begin_session({ accountId, origin, hasUserActivation })`, and propagate the returned `token` into `showInPagePicker` and every `screen_share_thumbnail({ token, id })` call so the session gate can authorise both halves of the picker flow. - On Share, Cancel, or the no-sources fallback path, finalize the session via a fire-and-forget `finalizeSessionQuiet` helper so the account's `active` slot is released immediately instead of waiting for the 30s TTL. - Expand the `navigator.permissions.query` spoof to also report `camera` and `microphone` as `granted`, not just `display-capture`. CEF Alloy's Permissions API otherwise reports them as `prompt`/ `denied`, and some Meet / Slack code paths short-circuit on that before ever calling `getUserMedia`. AbortError on cancel, the DOMContentLoaded retry, and the in-flight thumbnail dedup are all preserved. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…mansai#713) Addresses the shim-side half of graycyrus refactor note tinyhumansai#6 on tinyhumansai#809. The host session state already evicts a stale session when `begin_session` is called a second time for the same account, but without a shim-side guard a concurrent `getDisplayMedia` would still append a second picker DOM while the first overlay was live — the user would see two stacked cards and no clean way out of either. Tracks an `pickerInFlight` module-scoped boolean around the shim body (now factored into `runShim`) and rejects concurrent calls with `InvalidStateError`, which matches the MediaStreams spec's error shape for already-in-progress capture requests. The flag is reset in a `finally` block so exceptions from `runShim` don't leave it stuck. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
@graycyrus addressed your review in Blockers
Majors
Refactor
Nits
Residual risk (Stage A scope) The Tests 13 unit tests in @coderabbitai the new |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainThanks for the detailed summary, 🏁 Script executed: Length of output: 50977 Good progress verified — let me quickly check the permissions files and handler registration to make sure they align with the new session-gated command names. 🏁 Script executed: Length of output: 1739
✅ Verified
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src-tauri/src/screen_capture/mod.rs (2)
630-640: Consider guarding against negativeCFStringGetLengthreturn.
CFStringGetLengthreturnsCFIndex(signed), and while it should never be negative in practice, the cast tousizeat line 632 could produce unexpected results if it were. A defensive check would be:🛡️ Optional defensive guard
let len = CFStringGetLength(cf); + if len < 0 { + return None; + } // UTF-8 safety margin: 4 bytes per codepoint + NUL. let cap = (len as usize) * 4 + 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/screen_capture/mod.rs` around lines 630 - 640, CFStringGetLength returns a signed CFIndex (variable len) and the code currently casts it directly to usize to compute cap and allocate buf, which could overflow if len is negative; add a defensive check right after calling CFStringGetLength (before computing cap) to return None (or otherwise handle) when len < 0, and only proceed to compute cap = (len as usize) * 4 + 1, allocate buf and call CFStringGetCString; reference CFStringGetLength, CFStringGetCString, len, cap and buf to locate the change.
482-909: Consider extracting macOS backend to a separate file.At
430 lines, the macOS backend module is substantial. Per coding guidelines preferring files ≤500 lines, consider movingmod macostoscreen_capture/macos.rsto improve navigability. Not urgent given the logical cohesion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/screen_capture/mod.rs` around lines 482 - 909, The macOS backend module (mod macos) is large and should be moved into its own file: create screen_capture/macos.rs containing the entire contents of the current #[cfg(target_os = "macos")] mod macos { ... } block (keeping all types, extern declarations, constants, and functions like thumbnail_for_id, enumerate, enumerate_screens, enumerate_windows, screen_thumbnail_b64, window_thumbnail_b64, cgimage_to_png_bytes, cfstr, cfstring_to_string, cfnumber_to_u64, and the CoreGraphics/CF FFI), preserve the #[cfg(target_os = "macos")] attribute on the module declaration, and in the original mod.rs replace the whole block with a single #[cfg(target_os = "macos")] mod macos; ensure the new file keeps use super::ScreenSource and any visibility (pub(super)) so existing callers compile unchanged and run cargo build/test to verify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src-tauri/src/screen_capture/mod.rs`:
- Around line 630-640: CFStringGetLength returns a signed CFIndex (variable len)
and the code currently casts it directly to usize to compute cap and allocate
buf, which could overflow if len is negative; add a defensive check right after
calling CFStringGetLength (before computing cap) to return None (or otherwise
handle) when len < 0, and only proceed to compute cap = (len as usize) * 4 + 1,
allocate buf and call CFStringGetCString; reference CFStringGetLength,
CFStringGetCString, len, cap and buf to locate the change.
- Around line 482-909: The macOS backend module (mod macos) is large and should
be moved into its own file: create screen_capture/macos.rs containing the entire
contents of the current #[cfg(target_os = "macos")] mod macos { ... } block
(keeping all types, extern declarations, constants, and functions like
thumbnail_for_id, enumerate, enumerate_screens, enumerate_windows,
screen_thumbnail_b64, window_thumbnail_b64, cgimage_to_png_bytes, cfstr,
cfstring_to_string, cfnumber_to_u64, and the CoreGraphics/CF FFI), preserve the
#[cfg(target_os = "macos")] attribute on the module declaration, and in the
original mod.rs replace the whole block with a single #[cfg(target_os =
"macos")] mod macos; ensure the new file keeps use super::ScreenSource and any
visibility (pub(super)) so existing callers compile unchanged and run cargo
build/test to verify.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed09db0b-8a69-475a-bbb3-d99e9edb9a21
📒 Files selected for processing (6)
app/src-tauri/permissions/allow-core-process.tomlapp/src-tauri/permissions/allow-webview-recipe.tomlapp/src-tauri/src/lib.rsapp/src-tauri/src/screen_capture/mod.rsapp/src-tauri/src/webview_accounts/runtime.jsdocs/src-tauri/02-commands.md
✅ Files skipped from review due to trivial changes (2)
- docs/src-tauri/02-commands.md
- app/src-tauri/src/webview_accounts/runtime.js
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src-tauri/src/lib.rs
- app/src-tauri/permissions/allow-core-process.toml
- app/src-tauri/permissions/allow-webview-recipe.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
navigator.mediaDevices.getDisplayMedianow see a native-feeling OpenHuman picker with real screen / window thumbnails instead of silently capturing the entire primary display.begin_session(gated on caller label + user-activation claim + rate limit) before it can enumerate sources or fetch thumbnails, and a valid token only authorises thumbnails for IDs that were in the picker the user was actually shown."not implemented yet"until the design is proven end-to-end.Problem
tinyhumansai/openhuman#713AC2 — "Click 'Present now' → OS screen/window picker appears → user selects a screen or window → share streams into the Meet call".Our child webviews run under the CEF Alloy runtime, which does not link Chromium's built-in
DesktopMediaPicker(that code lives in//chrome/browserand Alloy strips it). With only the permission-handler grant in place, Chromium auto-selects the primary display and the user's entire screen silently streams into the call with no visible picker or consent flow.Solution
Three parts.
1. Native source enumerator + thumbnail capture (
screen_capture/)CoreGraphics + CoreFoundation + ImageIO FFI (no new transitive crate tree). Screens via
CGGetActiveDisplayList, windows viaCGWindowListCopyWindowInfo, thumbnails viaCGDisplayCreateImage/CGWindowListCreateImage→CGImageDestinationCreate{WithData,AddImage,Finalize}, base64-encoded for IPC. Thumbnails are captured on demand rather than during enumeration so the picker opens instantly.Source IDs are formatted as
screen:<CGDirectDisplayID>:0/window:<CGWindowID>:0— Chromium'sDesktopMediaID::Parsereads these directly, so the shim can hand them straight togetUserMediawithout going through the extension-onlychrome.desktopCaptureAPI.2. In-page getDisplayMedia shim (
webview_accounts/runtime.js)Installed via the existing initialization-script path for every account webview. Overrides
MediaDevices.prototype.getDisplayMedia(instance assignment is non-writable in modern Chromium, so weObject.definePropertyon the prototype), builds the picker overlay imperatively (Trusted Types CSP rejectsinnerHTMLon Meet), and on Share callsgetUserMediawith a hand-craftedchromeMediaSource: 'desktop' + chromeMediaSourceIdconstraint. Our CEF permission callback grantsDESKTOP_VIDEO_CAPTUREbits, so Chromium honours the ID and opens a real capture device.Patches the returned
MediaStreamTrack.getSettings()to exposedisplaySurface+contentHint = 'detail'— without that, Meet rejects the stream with a generic "Can't share your screen" error because the track looks indistinguishable from agetUserMediavideo source. Cancel throwsAbortErrorso Meet silently dismisses (instead of "Meet needs permission to screen share").navigator.permissions.queryis shadowed to reportdisplay-capture/camera/microphoneasgrantedso sites that front-check the Permissions API before callinggetUserMediadon't short-circuit. If the page requests audio, shim tries audio+video and falls back to video-only on rejection (stock macOS has no loopback audio driver).3. Session gating (#812 Stage A)
The first-pass landing exposed
screen_share_list_sources/screen_share_thumbnaildirectly on the recipe-webview allowlist, which let any script running inside an embedded site (page JS, compromised third-party CDN) silently enumerate every open window title + live thumbnail with no user gesture. CodeRabbit + graycyrus flagged this as a blocker on the first push.The commands are now session-gated.
screen_share_begin_sessionreturns{ token, sources }after enforcing:acct_(rejects main/overlay windows and any non-recipe webview).has_user_activationclaim from the shim is true (frontend readsnavigator.userActivation.isActive).InvalidStateError).screen_share_thumbnail(token, id)requires a live token and anidthat was in that session's enumerated source list — a leaked token can only replay thumbnails for IDs the shim already showed the user.screen_share_finalize_sessionis called on Share/Cancel to release the session immediately instead of waiting for the 30s TTL.Residual risk (honest scope of Stage A)
The
has_user_activationflagbegin_sessionreads is page-trusted. A Meet-origin script that lies withhasUserActivation: truestill bypasses the gesture gate — its only remaining bounds areacct_*caller + 10/min rate limit + thumbnail IDs restricted to what the picker actually showed. Host-verified gating needs theOnRequestMediaAccessPermissionhook in vendored tauri-cef (Stage B); that's tracked for a follow-up PR in #812. Stage A kills drive-by enumeration and cross-webview abuse cleanly; in-gesture malicious first-party JS is not closed by it.Submission Checklist
screen_capture/mod.rs::tests:parse_source_idhappy paths + 5 error shapes, sessionpurge_expired, rate-limit 11th-call block, rate-limit per-account scoping,generate_tokenURL-safe shape + uniqueness,token_prefixtruncation.getDisplayMediacall rejected cleanly).//!module preamble onscreen_capture/mod.rsexplains the flow end-to-end including the Stage A gating rationale; shim carries a long header block. New commands are documented indocs/src-tauri/02-commands.md.u32::try_fromoverflow reasoning, Sequoia TCC quirk,cfstrUB avoidance, each carry a short grep-friendly comment.Impact
"not implemented yet". The shim itself is cross-platform JavaScript, so as soon as a Windows/Linux enumerator lands, the picker works there too.chromeMediaSource: 'desktop'constraint path is normally gated to Chromium extensions. We accept the exposure deliberately because the permission grant still flows through our CEFOnRequestMediaAccessPermissionhandler, and the session-gated command API bounds what page JS can observe outside of a picker interaction. Residual risk documented above.Related
tinyhumansai/openhuman#713(AC2 — Meet screen share picker). Addresses blockers + review points raised on the first push by CodeRabbit and @graycyrus.tauri-cefOnRequestMediaAccessPermissionhook (closes thehas_user_activationpage-trust residual). Windows/Linux source enumerators. Optional per-provider stream-content-hint tuning.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
getDisplayMediaintegration with session-based access control and user gesture validation.Documentation