Skip to content

Commit 262d92d

Browse files
oxoxDevsenamakel
andauthored
fix(webview/slack): media perms + deep-link isolation (tinyhumansai#1074) (tinyhumansai#1080)
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
1 parent c7c9c62 commit 262d92d

3 files changed

Lines changed: 188 additions & 5 deletions

File tree

app/src-tauri/src/cdp/session.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,28 @@ async fn run_session_cycle<R: Runtime>(
309309
]);
310310
}
311311

312+
// Slack Huddles need the same media-capture set as Meet:
313+
// - audioCapture / videoCapture: getUserMedia for huddle voice +
314+
// optional camera tile. Without these, the huddle pre-flight
315+
// enumerateDevices returns empty and the join button silently
316+
// no-ops.
317+
// - displayCapture: getDisplayMedia for in-huddle screen share.
318+
// - clipboardReadWrite: huddle invite-link copy + slash-command
319+
// paste flows.
320+
// Mirrors the gmeet pattern from #1054. The huddle popup paint
321+
// lifecycle bug is tracked separately under #1074 / the CEF
322+
// tracking issue — granting these perms now means once the paint
323+
// bug clears, the huddle is functional immediately rather than
324+
// requiring a follow-up perms wire-up.
325+
if origin_host_is(&origin, "app.slack.com") {
326+
perms.extend_from_slice(&[
327+
"audioCapture",
328+
"videoCapture",
329+
"displayCapture",
330+
"clipboardReadWrite",
331+
]);
332+
}
333+
312334
if let Err(e) = cdp
313335
.call(
314336
"Browser.grantPermissions",
@@ -466,6 +488,29 @@ mod tests {
466488
assert!(!origin_host_is("file:///etc/hosts", "meet.google.com"));
467489
}
468490

491+
/// The slack-huddle media-perm grant is host-gated by
492+
/// `origin_host_is(origin, "app.slack.com")`. Lock the matcher so a
493+
/// future refactor can't silently widen / narrow the set of origins
494+
/// that get `audioCapture`/`videoCapture`/`displayCapture` etc.
495+
#[test]
496+
fn origin_host_is_matches_app_slack_com_for_huddle_grant() {
497+
// canonical slack web origin
498+
assert!(origin_host_is("https://app.slack.com", "app.slack.com"));
499+
// case-insensitive (matches Url-normalised input + raw header)
500+
assert!(origin_host_is("https://APP.SLACK.COM", "app.slack.com"));
501+
// explicit port tolerated
502+
assert!(origin_host_is("https://app.slack.com:443", "app.slack.com"));
503+
504+
// marketing site / files CDN must NOT receive media perms — only
505+
// the huddle-bearing app origin
506+
assert!(!origin_host_is("https://slack.com", "app.slack.com"));
507+
assert!(!origin_host_is("https://files.slack.com", "app.slack.com"));
508+
// unrelated provider
509+
assert!(!origin_host_is("https://meet.google.com", "app.slack.com"));
510+
// non-http schemes never match (e.g. about:blank popup placeholder)
511+
assert!(!origin_host_is("about:blank", "app.slack.com"));
512+
}
513+
469514
#[test]
470515
fn target_match_accepts_placeholder_and_real_provider_fragments_only_for_same_account() {
471516
assert!(target_matches_account_url(

app/src-tauri/src/webview_accounts/mod.rs

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,30 @@ fn popup_should_stay_in_app(provider: &str, url: &Url) -> bool {
225225
}
226226
}
227227

228+
/// `true` if `scheme` is a known provider native-desktop-app deep-link
229+
/// scheme. We suppress these instead of routing them to the system
230+
/// browser because macOS hands them to the native provider app
231+
/// (e.g. `slack://magic-login/<token>` signs the native Slack app into
232+
/// the workspace, breaking embedded-webview isolation: the workspace's
233+
/// session ends up inside the native client even though the user only
234+
/// signed in via OpenHuman's embedded webview).
235+
///
236+
/// The HTTPS fallback in each provider's web flow handles sign-in
237+
/// without the deep link, so suppression is safe — the page just
238+
/// continues on the next link in the sequence.
239+
///
240+
/// Caller contract: only suppress when [`rewrite_provider_deep_link`]
241+
/// has already returned `None` for the URL. Schemes we DO know how to
242+
/// rewrite into a web-client URL (e.g. `zoomus://`) must take the
243+
/// rewrite path first; those flows expect to stay in-app, not be
244+
/// silently dropped.
245+
fn is_provider_native_deep_link_scheme(scheme: &str) -> bool {
246+
matches!(
247+
scheme,
248+
"slack" | "discord" | "tg" | "msteams" | "zoomus" | "zoommtg"
249+
)
250+
}
251+
228252
/// `true` if a popup request should be denied AND the parent webview
229253
/// should be navigated to the popup URL instead.
230254
///
@@ -301,6 +325,10 @@ fn redact_navigation_url(url: &Url) -> String {
301325
safe.set_fragment(None);
302326
safe.to_string()
303327
}
328+
329+
fn redact_native_deep_link_url(url: &Url) -> String {
330+
format!("{}://<redacted>", url.scheme())
331+
}
304332
/// Unwrap provider-side "link safety" redirects so the system browser
305333
/// lands on the real destination.
306334
///
@@ -1554,6 +1582,20 @@ pub async fn webview_account_open<R: Runtime>(
15541582
if url_is_internal(&nav_provider, url) {
15551583
true
15561584
} else {
1585+
// Suppress provider native-desktop-app deep-link schemes that
1586+
// we don't know how to rewrite. macOS would otherwise hand
1587+
// these to the native provider app — `slack://magic-login/…`
1588+
// signs the native Slack app into the workspace, breaking
1589+
// embedded-webview isolation (#1074). The web flow's HTTPS
1590+
// fallback handles sign-in without the deep link.
1591+
if is_provider_native_deep_link_scheme(url.scheme()) {
1592+
log::warn!(
1593+
"[webview-accounts] suppressing native-app deep-link scheme={} url={} (would breach workspace isolation)",
1594+
url.scheme(),
1595+
redact_native_deep_link_url(url)
1596+
);
1597+
return false;
1598+
}
15571599
let target = unwrap_provider_redirect(url)
15581600
.map(|u| u.to_string())
15591601
.unwrap_or_else(|| url.to_string());
@@ -1638,6 +1680,19 @@ pub async fn webview_account_open<R: Runtime>(
16381680
);
16391681
NewWindowResponse::Allow
16401682
} else {
1683+
// Suppress provider native-desktop-app deep-link schemes that
1684+
// we don't know how to rewrite (matches the on_navigation
1685+
// fallback). Without this, a `slack://...` popup would land
1686+
// in the native Slack app via macOS's URL handler and
1687+
// breach embedded-webview workspace isolation (#1074).
1688+
if is_provider_native_deep_link_scheme(url.scheme()) {
1689+
log::warn!(
1690+
"[webview-accounts] suppressing native-app deep-link scheme={} url={} (would breach workspace isolation)",
1691+
url.scheme(),
1692+
redact_native_deep_link_url(&url)
1693+
);
1694+
return NewWindowResponse::Deny;
1695+
}
16411696
let target = unwrap_provider_redirect(&url)
16421697
.map(|u| u.to_string())
16431698
.unwrap_or_else(|| url.to_string());
@@ -2732,6 +2787,87 @@ mod tests {
27322787
.is_none());
27332788
}
27342789

2790+
// ── is_provider_native_deep_link_scheme: native-app suppression ───
2791+
//
2792+
// These guard the workspace-isolation contract from #1074: provider
2793+
// native-desktop-app deep-link schemes must NEVER reach the system
2794+
// browser, because macOS hands them off to the native provider app
2795+
// which then signs the user into the workspace using session tokens
2796+
// intended only for the embedded webview (see slack://magic-login
2797+
// smoking gun in the #1074 trace).
2798+
2799+
#[test]
2800+
fn deep_link_scheme_matches_known_provider_native_apps() {
2801+
// Slack desktop ("slack://T01.../magic-login/<token>")
2802+
assert!(is_provider_native_deep_link_scheme("slack"));
2803+
// Discord desktop
2804+
assert!(is_provider_native_deep_link_scheme("discord"));
2805+
// Telegram desktop ("tg://join?invite=…")
2806+
assert!(is_provider_native_deep_link_scheme("tg"));
2807+
// Microsoft Teams
2808+
assert!(is_provider_native_deep_link_scheme("msteams"));
2809+
// Zoom client (both variants registered by the installer)
2810+
assert!(is_provider_native_deep_link_scheme("zoomus"));
2811+
assert!(is_provider_native_deep_link_scheme("zoommtg"));
2812+
}
2813+
2814+
#[test]
2815+
fn deep_link_scheme_rejects_legitimate_external_schemes() {
2816+
// HTTP(S) — the bread-and-butter external link.
2817+
assert!(!is_provider_native_deep_link_scheme("https"));
2818+
assert!(!is_provider_native_deep_link_scheme("http"));
2819+
// Mail clients are legit external — must NOT be suppressed.
2820+
assert!(!is_provider_native_deep_link_scheme("mailto"));
2821+
// Telephone / sms are legit external too.
2822+
assert!(!is_provider_native_deep_link_scheme("tel"));
2823+
assert!(!is_provider_native_deep_link_scheme("sms"));
2824+
// about: / data: / blob: handled elsewhere; never deep-link.
2825+
assert!(!is_provider_native_deep_link_scheme("about"));
2826+
assert!(!is_provider_native_deep_link_scheme("data"));
2827+
assert!(!is_provider_native_deep_link_scheme("blob"));
2828+
// Empty / unrelated string.
2829+
assert!(!is_provider_native_deep_link_scheme(""));
2830+
assert!(!is_provider_native_deep_link_scheme("file"));
2831+
}
2832+
2833+
#[test]
2834+
fn deep_link_scheme_matches_real_world_slack_magic_login_url() {
2835+
// Real slack://-flavoured magic-login URL recorded in the
2836+
// #1074 CDP trace. The handler must catch it before
2837+
// open_in_system_browser is reached.
2838+
let parsed = url("slack://T01CWHNCJ9Z/magic-login/11035712490054-abc");
2839+
assert!(is_provider_native_deep_link_scheme(parsed.scheme()));
2840+
}
2841+
2842+
#[test]
2843+
fn deep_link_scheme_does_not_match_https_app_slack_com() {
2844+
// The web-flow URL stays untouched — only the slack:// scheme is
2845+
// suppressed; ordinary HTTPS slack navigations route normally.
2846+
let parsed = url("https://app.slack.com/client/T01CWHNCJ9Z");
2847+
assert!(!is_provider_native_deep_link_scheme(parsed.scheme()));
2848+
}
2849+
2850+
/// Locks the contract that zoomus:// stays on the rewrite path
2851+
/// (handled by `rewrite_provider_deep_link` for the "zoom" provider)
2852+
/// rather than being silently suppressed.
2853+
///
2854+
/// The wiring in on_navigation / on_new_window calls
2855+
/// `rewrite_provider_deep_link` BEFORE the suppress check, so a
2856+
/// rewriteable scheme is rewritten and never reaches the suppress
2857+
/// branch. This test pins both halves of that contract: the rewrite
2858+
/// still succeeds for zoom, AND the scheme is recognised as a
2859+
/// native-app deep-link (so if a future provider config dropped the
2860+
/// rewrite, suppression would be the safe fallback rather than
2861+
/// leaking to the system browser).
2862+
#[test]
2863+
fn zoomus_join_still_rewrites_and_is_recognized_as_native_scheme() {
2864+
let zoom_url = url("zoomus://zoom.us/join?action=join&confno=9819254358");
2865+
assert!(is_provider_native_deep_link_scheme(zoom_url.scheme()));
2866+
let rewritten = rewrite_provider_deep_link("zoom", &zoom_url)
2867+
.expect("zoom rewrite should still succeed before suppress branch");
2868+
assert_eq!(rewritten.as_str(), "https://app.zoom.us/wc/join/9819254358");
2869+
}
2870+
27352871
#[test]
27362872
fn rewrite_percent_encodes_reserved_chars_in_pwd() {
27372873
// Zoom tokens commonly contain `&` / `=` / `%` / `#` / `+` which

0 commit comments

Comments
 (0)