Make in-container OAuth logins actually complete #57

Merged

jknapp merged 9 commits from fix/auth-callback-and-opener into main 2026-09-18 04:32:07 +00:00

Conversation 1 Commits 9 Files Changed 28

+2367 -169

9 Commits

Author	SHA1	Message	Date
shadowdaoandClaude Opus 5	943c83b9e3	fix: stop a stale payload re-enabling a bridge the user turned off ... Review of this branch found that `update_project` restored `browser_view_enabled` from the store but took `auth_bridge_enabled` from the IPC payload, on a comment claiming the Config tab edits it through that save. The comment was wrong. `AuthBridgeRow` is the only writer, it calls `set_auth_bridge_enabled` out of band precisely so the switch works while a login is hanging, and it never writes the value back into frontend state -- so a payload's copy of that flag is always a stale snapshot. The consequence was not cosmetic: turn the bridge off, then close a renamed terminal tab, and `useTerminal` round-trips the stale `true` and the reconcile block restarts a bridge whose own UI warns that a bridged port is unauthenticated and reachable by any local process. Defaulting the flag to true earlier in this branch made it worse, since the stale value is now true for every pre-existing project. Both flags are now restored from the store by `restore_store_owned_fields`, and the reconcile block is gone rather than corrected: with the value always restored it could only re-assert what was already true, and every writer already owns its own side effect -- the setter starts and stops synchronously, container start arms the bridge, launch reconcile re-arms it, and the poller re-reads the flag each tick and self-terminates. Re-adding a start path to the one function that no longer owns the flag is what caused this. Turning the browser view off also stopped tearing the session down when the project record had vanished, because the persist used `?` and returned early -- the supervisor's own `store.get()` check exists because records do vanish mid-session. Teardown is now unconditional and the write error still surfaces afterwards, since the stored flag saying "enabled" means the view returns on next launch and that is worth reporting. Finally, the opener no longer falls through to `gio` on any non-zero exit. xdg-open's 1, 2 and 3 assert no handler ran; 4 also covers a handler that was launched and then failed, which would have opened the link twice -- two authorize requests for one click in an OAuth flow. Reasoned from documented exit codes rather than an observed double-open, and the cost is stated: a genuine code-4 failure no longer reaches gio. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 11:13:57 -07:00
shadowdaoandClaude Opus 5	60188610ee	fix: do not let an in-flight open blank a newer prompt, or promise a bridge that is off ... Two findings from review of this branch. Awaiting the open instead of dismissing up front bought a window: on Linux it is at least OPENER_GRACE, doubled when xdg-open fails and gio is tried. If the container relays a second URL inside that window, the first open's resolution blanked the second prompt -- losing a link that exists only in the container's transcript, which is the failure "dismiss on success only" was made to prevent. The slot already carried a `seq` for exactly this reason; dismissal is now conditional on it. `urlPromptRef` is written eagerly by the two functions that change the slot rather than synced by an effect. That is load-bearing: an effect-synced mirror lags state by a commit, and a promise microtask can resolve between `setUrlPrompt` and React flushing passive effects -- so it answers "did a newer prompt land?" wrong in precisely the window the guard exists for. Dropping the functional updater also fixes `promptSeqRef.current += 1` being mutated inside a state updater React is free to invoke twice. The guard is a sibling function rather than an optional argument on `dismissUrlPrompt`, because that function is passed by reference as UrlToast's `onDismiss` and React would hand it a MouseEvent as its first argument -- the seq check would fail and the close button would silently stop working, with the types still assignable. Separately, the sign-in hint was binary on which button leads, but "host leads" covers both a live bridge and a fallback where nothing is set up to catch the callback at all. In the second case the toast promised the bridge would carry it and the login hung to its timeout. The target is now three-state, the hint tells the truth in the fallback case and names the control that fixes it, and the hook starts at `host-fallback` rather than assuming a bridge it has not confirmed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 11:12:22 -07:00
shadowdaoandClaude Opus 5	db648230ee	chore: regenerate capabilities schema after dropping the opener grant ... Tracked build output; regenerated by the Tauri build from capabilities/default.json. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 10:11:03 -07:00
shadowdaoandClaude Opus 5	5a452e7a2a	security: drop opener:allow-open-url now that nothing calls it ... default.json carried this grant with an explicit accepted residual risk: a compromised webview could make the OS open an attacker-chosen http(s) URL. It was accepted because it could not be narrowed -- WebLinksAddon opens links Claude printed inside the container, which are arbitrary by construction, so a host allowlist would have deleted the feature. Now that every host-browser open routes through `open_url_external`, the webview has no reason to reach the plugin directly, and the risk closes rather than stays recorded. The plugin remains a dependency: macOS and Windows still use it, through `OpenerExt::open_url`, whose desktop implementation calls `crate::open::open` directly and is not gated by capabilities at all (tauri-plugin-opener-2.5.3/src/lib.rs:60) -- verified rather than assumed, since the whole point is that the Rust path keeps working. What is removed is the webview's ability to reach the opener without passing the Rust-side validation. The census note in default.json is rewritten to match, and lib.rs's grant-list test is updated deliberately, as its own assertion message demands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 10:10:45 -07:00
shadowdaoandClaude Opus 5	5a09254538	fix: route every host-browser open through open_url_external ... The Rust command existed but nothing called it. All four frontend call sites still used `openUrl` from `@tauri-apps/plugin-opener`, so the environment fix was inert and the three dialogs carried the same Linux bug as the terminal: DockerInstallDialog's docs link, ClaudeAuthModal's sign-in link and UpdateDialog's release link would all have reported success while launching nothing. `openUrlExternal` in tauri-commands.ts is now the single sink. There is no platform branch: Linux gets the sanitized spawn, macOS and Windows reach the same plugin as before but from Rust, and every platform picks up the Rust-side re-validation, which matters because these URLs originate in an untrusted container. Comments in urlRelay.ts and urlDetector.ts that named `openUrl` as the sink they guard are updated to match, and the two test files that mocked `@tauri-apps/plugin-opener` now mock the command instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 10:09:57 -07:00
shadowdaoandClaude Opus 5	9297020688	fix: open host URLs with a clean environment (triple-c#34) ... On Linux the app ships as a single AppImage, and the AppImage environment leaks into everything it spawns. linuxdeploy's AppRun, linuxdeploy-plugin-gtk and our own wayland fallback hook all export LD_LIBRARY_PATH, GTK_PATH, GIO_MODULE_DIR and friends pointing inside $APPDIR, and main.rs sets WEBKIT_DISABLE_DMABUF_RENDERER process-wide for the webview. A browser that is already running shrugs this off, because xdg-open just hands the URL to the existing process. A cold-launched one inherits the lot and dies before painting -- with xdg-open still exiting 0, which is why this looked like the button doing nothing at all. `url_open` captures a pristine snapshot of the environment in main() before any mutation runs, then hands children a repaired copy: a saved original is restored where one exists, otherwise the process-start value is restored where we changed it, otherwise only the colon-separated entries that live under $APPDIR are dropped and the user's own are kept. Outside an AppImage it is a no-op. The command re-validates the URL in Rust rather than trusting the frontend, because the URL originates in an untrusted container: http/https only, no embedded credentials, no control characters or whitespace, length capped, ASCII asserted before it reaches execvp, and error messages never echo the input. Spawning is Command with explicit args and never a shell, trying xdg-open then gio open. No portal. org.freedesktop.portal.OpenURI would pull in a D-Bus client stack for one call on the one platform where we ship self-contained, and it only helps where a portal is running -- the same case where xdg-open already works once the environment is clean. `gio open` as a second candidate recovers most of the missing-MIME-association case for free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 10:08:20 -07:00
shadowdaoandClaude Opus 5	bf8094dbc4	fix: route sign-in links by what can actually catch the callback ... `isAnthropicSignInUrl` made the container the default action for every Anthropic sign-in link, justified by "the host has nothing to catch it with". That was wrong in both directions. The host does have something -- the auth bridge -- and the container side is not a general browser at all but Playwright's dashboard, whose packages and chromium are deliberately not baked into the image. So the default pointed at the one path that is uninstalled on a fresh project, on every platform, while the path that works sat behind a switch. The decision now lives in `useSignInOpenTarget`: a live auth bridge picks the host, otherwise a container that can actually launch a browser picks the container, otherwise the host. It resolves at mount rather than when a URL arrives, so the buttons do not swap under a moving mouse, and it re-decides on `auth-bridge-changed` so flipping the switch during a hanging login takes effect. A bridge with port conflicts reads as not live; an empty `active_ports` does not, since there is nothing to bridge until the CLI binds its listener and that races the URL. Both buttons still render either way -- this changes which one leads. `sanitizeRelayUrl` is byte-for-byte unchanged, so the embedded copy in web_terminal/terminal.html needs no matching edit. The host "Open" path also failed silently: `dismissUrlPrompt()` ran before `openUrl`, so the toast vanished and a rejected promise reached only the devtools console. Dismissal now happens on success only, leaving "In container" one click away after a failure, and the error surfaces through the same toast the container path already used. On Linux this catch will not fire for the common case -- `xdg-open` routinely exits 0 having done nothing -- so it complements the AppImage environment fix rather than replacing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 10:07:50 -07:00
shadowdaoandClaude Opus 5	90b7e4ccb2	fix: default the auth bridge on, and make the browser-view flag durable ... A CLI running `claude login` inside the container binds a random ephemeral loopback port and hands the provider a redirect pointing at it. The browser is on the host, so the callback lands on a closed host port and the login hangs with no diagnostic. The auth bridge is the thing that fixes this -- it mirrors container loopback listeners onto the same host port -- so having it default to off made a hang the out-of-the-box experience. `auth_bridge_enabled` now defaults to true through a `default_auth_bridge_enabled()` serde helper, matching the shape already used by `use_shared_auth_token`. Because the default is applied at deserialisation, projects stored before the bridge existed pick it up too; `migrate_from_value` writes neither flag, so nothing defeats it, and a regression test pins that. Separately, `BrowserViewManager.enabled` was in-memory only and the durable `browser_view_enabled` field on the project record was never implemented. Rather than sync the two, the cache is removed and the record becomes the single home for the flag, mirroring how `AuthBridgeManager` already works. `stop()` deliberately does not clear it, since container teardown and migration reach that path and neither is the user changing their mind. Durable does not mean auto-started: a restarted app reports enabled with the viewer off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 10:06:09 -07:00
shadowdaoandClaude Opus 5	afe9d5cdb2	docs: correct Linux packaging in BUILDING.md ... BUILDING.md listed AppImage, .deb and .rpm as build artifacts, but Linux ships as AppImage only -- CI passes `--bundles appimage`, and the .deb and .rpm were dropped because neither could self-update. A bare `npx tauri build` still emits all three, since tauri.conf.json keeps "targets": "all" to leave macOS and Windows untouched, so the table now marks which are actually released rather than pretending the others do not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 10:01:29 -07:00