Compare commits
12
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
943c83b9e3 | ||
|
|
60188610ee | ||
|
|
db648230ee | ||
|
|
5a452e7a2a | ||
|
|
5a09254538 | ||
|
|
9297020688 | ||
|
|
bf8094dbc4 | ||
|
|
90b7e4ccb2 | ||
|
|
afe9d5cdb2 | ||
|
|
b59c6148ff | ||
|
|
95a78fe9a3 | ||
|
|
307ea07409 |
+21
-5
@@ -71,13 +71,29 @@ npm ci
|
|||||||
npx tauri build
|
npx tauri build
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Linux ships as **AppImage only**. To match what CI produces, pass the bundle
|
||||||
|
explicitly:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
npx tauri build --bundles appimage
|
||||||
|
```
|
||||||
|
|
||||||
|
The `.deb` and `.rpm` bundles were dropped — two more artifacts to build and
|
||||||
|
publish for an audience the AppImage already serves, and neither could
|
||||||
|
self-update. A bare `npx tauri build` still emits them, because
|
||||||
|
`tauri.conf.json` keeps `"targets": "all"` so that macOS and Windows are
|
||||||
|
untouched; they are not released and not tested.
|
||||||
|
|
||||||
Build artifacts are located in `app/src-tauri/target/release/bundle/`:
|
Build artifacts are located in `app/src-tauri/target/release/bundle/`:
|
||||||
|
|
||||||
| Format | Path |
|
| Format | Path | Released |
|
||||||
|------------|-------------------------------|
|
|------------|-------------------------------|----------|
|
||||||
| AppImage | `appimage/*.AppImage` |
|
| AppImage | `appimage/*.AppImage` | yes |
|
||||||
| Debian pkg | `deb/*.deb` |
|
| Debian pkg | `deb/*.deb` | no |
|
||||||
| RPM pkg | `rpm/*.rpm` |
|
| RPM pkg | `rpm/*.rpm` | no |
|
||||||
|
|
||||||
|
`scripts/finalize-appimage.sh` post-processes the AppImage; see the Packaging
|
||||||
|
section of `CLAUDE.md` for why both of its steps are load-bearing.
|
||||||
|
|
||||||
## macOS
|
## macOS
|
||||||
|
|
||||||
|
|||||||
@@ -456,6 +456,63 @@ security update. Migration is the non-destructive way out; Reset is the destruct
|
|||||||
bump: churn on the old base, and it would consume the "you should migrate" signal without
|
bump: churn on the old base, and it would consume the "you should migrate" signal without
|
||||||
migrating. `get_container_staleness` surfaces it; `migrate_project_to_base` acts on it.
|
migrating. `get_container_staleness` surfaces it; `migrate_project_to_base` acts on it.
|
||||||
- **A missing lineage label means "unknown, probe instead", never "stale".**
|
- **A missing lineage label means "unknown, probe instead", never "stale".**
|
||||||
|
- **The snapshot image is not a checkpoint — never read its absence as "nothing to inspect".**
|
||||||
|
`commit_container_snapshot` runs only before a container is destroyed (a config-change recreate)
|
||||||
|
or inside a migration. **Never on stop.** So a project in daily use for a year can legitimately
|
||||||
|
have no `triple-c-snapshot-{id}:latest` at all, and one that has is stale by everything installed
|
||||||
|
since. `pick_probe_source` therefore reads a *stopped* container directly — commit its writable
|
||||||
|
layer to a unique `triple-c-probe-*` image, probe that, drop it — and ranks it **above** the snapshot,
|
||||||
|
for the same reason a running container already outranked it. Assuming a snapshot existed is what
|
||||||
|
made a stopped, never-recreated project report "no container or snapshot image yet" with its
|
||||||
|
container sitting right there, and left Update disabled on the projects furthest behind.
|
||||||
|
- **`bollard` never gives you the image id back from a commit.** Its `Commit` response model
|
||||||
|
deserialises `"ID"`; the daemon sends `"Id"`, so `commit_container` returns `id: None` every time
|
||||||
|
(verified: bollard 0.18.1, Engine 29.6). Neither long-standing commit site notices because both
|
||||||
|
discard the response — but it means any commit you need a *reference* to has to be **tagged**.
|
||||||
|
- **A tagged leftover is the one orphan no sweep can reach, so the probe image has its own reaper.**
|
||||||
|
`sweep_orphaned_snapshots` collects `dangling` + `triple-c.managed=true`; `reap_stale_migration_pins`
|
||||||
|
and `scrub_secrets_from_snapshots` both filter `triple-c-snapshot-*`. A `triple-c-probe-*` image is
|
||||||
|
tagged and so matches none of them, which would make a crashed probe a permanent multi-gigabyte
|
||||||
|
leak with no UI to find it. `reap_probe_images` runs at startup beside `reap_probe_containers` and
|
||||||
|
is **load-bearing, not tidying** — it is also what makes the probe image's unscrubbed writable
|
||||||
|
layer acceptable. Two rules it earned the hard way:
|
||||||
|
- **Age-gate it** (`PROBE_REAP_MIN_AGE_SECS`, same as the container reaper). `reference=` is
|
||||||
|
daemon-wide, so a second copy of the app has live probe images matching the glob.
|
||||||
|
- **Remove by tag, never by image id.** A `force` removal by id untags an image *everywhere*; a
|
||||||
|
fixture that tagged `alpine:latest` into this namespace deleted the user's alpine that way.
|
||||||
|
- **Probe image names are unique per call, and must stay that way.** A stable per-container name was
|
||||||
|
tried: container ids do not survive a recreate, so most leftovers were stranded permanently, and
|
||||||
|
two concurrent probes fought over one tag — whichever finished first force-removed the image the
|
||||||
|
other was still reading, reporting a bogus `probe_error` on a healthy project. `get_container_staleness`
|
||||||
|
takes no `project_lock` claim (the migration banner needs it to answer *during* a migration), so
|
||||||
|
uniqueness is what makes overlapping probes safe.
|
||||||
|
- **The stopped-container probe is cached per stop, and that is not an optimisation you may drop.**
|
||||||
|
`getContainerStaleness` is called from a `useEffect` that fires whenever the container settles, so
|
||||||
|
merely opening a stopped project's Overview probes it. Uncached that is a `docker commit` of the
|
||||||
|
whole writable layer per visit — measured at 44 s on a real project, against ~3 s for the snapshot
|
||||||
|
probe it replaced. `STOPPED_MANIFEST_CACHE` is keyed on the container's `FinishedAt`, which is
|
||||||
|
exact rather than merely plausible: nothing can write to a stopped container's writable layer, and
|
||||||
|
`FinishedAt` moves on every stop. A live test asserts the restart case, because a cache that
|
||||||
|
failed to invalidate would plan a migration against a filesystem the project no longer has.
|
||||||
|
- **Do not "skip the probe when the project is not stale" to save that cost.** It was tried. The
|
||||||
|
deltas would be empty while `probeSettled` (`!probing && staleness && !probe_error`) stayed *true*,
|
||||||
|
which leaves the migrate action in the project menu enabled — that action is not gated on the
|
||||||
|
banner — so the pre-flight would report nothing to copy while the backend was told to copy
|
||||||
|
nothing. That is the exact hazard `ProjectHome.tsx`'s `canMigrate` comment already warns about.
|
||||||
|
- **A failed stopped-container probe falls back to the snapshot whenever one exists.** Before this
|
||||||
|
feature a stopped project read its snapshot directly, so surfacing a commit failure where the
|
||||||
|
snapshot could have answered would make the banner *worse* than it was — and the failure modes are
|
||||||
|
exactly the ones where the fallback earns its keep: a full disk (the commit allocates the whole
|
||||||
|
writable layer; the snapshot probe allocates nothing) and a 409 from a concurrent claim.
|
||||||
|
- **`get_container_staleness` never commits while the project is claimed.** It takes no
|
||||||
|
`project_lock` claim itself, deliberately — the banner has to answer *during* a migration — so it
|
||||||
|
reads `project_lock::held` instead and probes the snapshot rather than the container. The
|
||||||
|
collision is not symmetric: the probe losing is a retryable `probe_error`, but
|
||||||
|
`start_project_container` removes the old container with a hard `?`, so a remove that raced a
|
||||||
|
commit would fail the user's Start with an opaque error.
|
||||||
|
- **An image's `Created` is the image's own, not its tag's.** Tagging an existing image gives you
|
||||||
|
that image's age; BuildKit stamps `docker build` output with a fixed epoch. Only `docker commit`
|
||||||
|
stamps *now* — which is what real probe images do, and what any fixture for them must do.
|
||||||
- **`:latest` keeps pointing at the old lineage until the final commit.** That is what makes every
|
- **`:latest` keeps pointing at the old lineage until the final commit.** That is what makes every
|
||||||
crash before that point self-heal — `start_project_container` just recreates from the old
|
crash before that point self-heal — `start_project_container` just recreates from the old
|
||||||
snapshot. After the container swap, the new container's `triple-c.migration-state=in-progress`
|
snapshot. After the container swap, the new container's `triple-c.migration-state=in-progress`
|
||||||
|
|||||||
Generated
+1
@@ -5306,6 +5306,7 @@ dependencies = [
|
|||||||
"tauri-plugin-opener",
|
"tauri-plugin-opener",
|
||||||
"tokio",
|
"tokio",
|
||||||
"tower-http",
|
"tower-http",
|
||||||
|
"url",
|
||||||
"uuid",
|
"uuid",
|
||||||
"zeroize",
|
"zeroize",
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -39,6 +39,10 @@ local-ip-address = "0.6"
|
|||||||
argon2 = "0.5"
|
argon2 = "0.5"
|
||||||
aes-gcm = "0.10"
|
aes-gcm = "0.10"
|
||||||
zeroize = "1"
|
zeroize = "1"
|
||||||
|
# WHATWG URL parsing for `url_open`'s re-validation of URLs arriving from the
|
||||||
|
# container. Already in the tree transitively (reqwest), and the point of
|
||||||
|
# using it rather than hand-rolling is parity with the frontend's `new URL()`.
|
||||||
|
url = "2"
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
# `test-util` (not part of tokio's `full`) lets the auto-start retry tests run
|
# `test-util` (not part of tokio's `full`) lets the auto-start retry tests run
|
||||||
|
|||||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -15,6 +15,12 @@ use crate::AppState;
|
|||||||
/// non-`Running` status carrying an explanation rather than an error, so the
|
/// non-`Running` status carrying an explanation rather than an error, so the
|
||||||
/// pane always has something specific to say. This is host-side only — no
|
/// pane always has something specific to say. This is host-side only — no
|
||||||
/// container recreation is involved either way.
|
/// container recreation is involved either way.
|
||||||
|
///
|
||||||
|
/// Either way the choice is persisted, so it survives an app restart. This is
|
||||||
|
/// the only caller allowed to write `false`: every other path to
|
||||||
|
/// [`BrowserViewManager::stop`](crate::browser_view::BrowserViewManager::stop)
|
||||||
|
/// is a teardown rather than the user changing their mind. Enabling persists
|
||||||
|
/// inside `start`, which is the single funnel for it.
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
pub async fn set_browser_view_enabled(
|
pub async fn set_browser_view_enabled(
|
||||||
project_id: String,
|
project_id: String,
|
||||||
@@ -23,9 +29,32 @@ pub async fn set_browser_view_enabled(
|
|||||||
state: State<'_, AppState>,
|
state: State<'_, AppState>,
|
||||||
) -> Result<BrowserViewStatus, String> {
|
) -> Result<BrowserViewStatus, String> {
|
||||||
if !enabled {
|
if !enabled {
|
||||||
|
// Persist first, then tear down: the supervisor's own teardown emit
|
||||||
|
// reads this flag back out of the store, and reading it mid-stop would
|
||||||
|
// announce a view that is going away as still enabled.
|
||||||
|
//
|
||||||
|
// But the write's outcome is a *value*, not a branch. A `?` here meant
|
||||||
|
// that a store with no such project record returned early and
|
||||||
|
// `manager().stop()` never ran, leaving the supervisor, the proxy and
|
||||||
|
// the host port up for a project that, as far as the user is concerned,
|
||||||
|
// just had its view switched off. That state is not hypothetical while
|
||||||
|
// a session is live — the supervisor's own `store.get()` check in
|
||||||
|
// [`crate::browser_view`] exists because a record can go away
|
||||||
|
// underneath it — and before the flag was persisted at all, turning the
|
||||||
|
// view off always tore the session down.
|
||||||
|
let persisted = state
|
||||||
|
.projects_store
|
||||||
|
.set_browser_view_enabled(&project_id, false);
|
||||||
// Awaits the supervisor, so the host port is released before we return.
|
// Awaits the supervisor, so the host port is released before we return.
|
||||||
manager().stop(&project_id).await;
|
//
|
||||||
return Ok(manager().status(&project_id).await);
|
// A failed write is still reported rather than logged and swallowed.
|
||||||
|
// The resources are gone either way by this point, so surfacing it
|
||||||
|
// costs nothing that matters, and the failure it describes is one the
|
||||||
|
// user needs: the stored flag still says *enabled*, so the view comes
|
||||||
|
// back by itself on the next launch. Returning `Ok` would be a claim
|
||||||
|
// about persistence that isn't true.
|
||||||
|
tear_down_then_report(persisted, manager().stop(&project_id)).await?;
|
||||||
|
return Ok(manager().status(&project_id, false).await);
|
||||||
}
|
}
|
||||||
|
|
||||||
let container_id = running_container(&state, &project_id, "opening the browser view").await?;
|
let container_id = running_container(&state, &project_id, "opening the browser view").await?;
|
||||||
@@ -40,10 +69,31 @@ pub async fn set_browser_view_enabled(
|
|||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Current status. Cheap: reads in-process state only, never the container.
|
/// Await `teardown`, then report `persisted`.
|
||||||
|
///
|
||||||
|
/// Trivial on purpose, and split out for one reason: it is the whole rule the
|
||||||
|
/// disable path of [`set_browser_view_enabled`] has to obey — the teardown is
|
||||||
|
/// unconditional, and a failed persist surfaces only after it has run — and as
|
||||||
|
/// a free function that rule can be tested without a live `AppState`.
|
||||||
|
async fn tear_down_then_report(
|
||||||
|
persisted: Result<(), String>,
|
||||||
|
teardown: impl std::future::Future<Output = ()>,
|
||||||
|
) -> Result<(), String> {
|
||||||
|
teardown.await;
|
||||||
|
persisted
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Current status. Cheap: the session map in this process plus the stored flag,
|
||||||
|
/// never the container.
|
||||||
|
///
|
||||||
|
/// The two are independent on purpose — this is what the pane reads on mount,
|
||||||
|
/// and after an app restart the honest answer is "enabled, nothing running".
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
pub async fn get_browser_view_status(project_id: String) -> Result<BrowserViewStatus, String> {
|
pub async fn get_browser_view_status(
|
||||||
Ok(manager().status(&project_id).await)
|
project_id: String,
|
||||||
|
state: State<'_, AppState>,
|
||||||
|
) -> Result<BrowserViewStatus, String> {
|
||||||
|
Ok(manager().status(&project_id, enabled_for(&state, &project_id)).await)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Probe the container for Playwright without starting anything.
|
/// Probe the container for Playwright without starting anything.
|
||||||
@@ -110,7 +160,9 @@ pub async fn open_browser_view_popout(
|
|||||||
app_handle: AppHandle,
|
app_handle: AppHandle,
|
||||||
state: State<'_, AppState>,
|
state: State<'_, AppState>,
|
||||||
) -> Result<(), String> {
|
) -> Result<(), String> {
|
||||||
let status = manager().status(&project_id).await;
|
let status = manager()
|
||||||
|
.status(&project_id, enabled_for(&state, &project_id))
|
||||||
|
.await;
|
||||||
let (BrowserViewState::Running, Some(url)) = (status.state, status.url.as_deref()) else {
|
let (BrowserViewState::Running, Some(url)) = (status.state, status.url.as_deref()) else {
|
||||||
return Err(
|
return Err(
|
||||||
"The browser view isn't running. Start it before opening it in its own window."
|
"The browser view isn't running. Start it before opening it in its own window."
|
||||||
@@ -209,7 +261,9 @@ pub async fn open_page_in_container_browser(
|
|||||||
// the user to go and press Start in the Browser tab themselves — and from
|
// the user to go and press Start in the Browser tab themselves — and from
|
||||||
// the terminal's URL prompt, with no indication that was even needed.
|
// the terminal's URL prompt, with no indication that was even needed.
|
||||||
// Asking for a page *is* asking to watch it, so the viewer comes up too.
|
// Asking for a page *is* asking to watch it, so the viewer comes up too.
|
||||||
let status = manager().status(&project_id).await;
|
let status = manager()
|
||||||
|
.status(&project_id, enabled_for(&state, &project_id))
|
||||||
|
.await;
|
||||||
if status.state != BrowserViewState::Running {
|
if status.state != BrowserViewState::Running {
|
||||||
crate::commands::project_commands::emit_progress(
|
crate::commands::project_commands::emit_progress(
|
||||||
&app_handle,
|
&app_handle,
|
||||||
@@ -229,7 +283,9 @@ pub async fn open_page_in_container_browser(
|
|||||||
// From the terminal there is no pane on screen to fill, so the page needs a
|
// From the terminal there is no pane on screen to fill, so the page needs a
|
||||||
// window of its own or it lands somewhere the user isn't looking.
|
// window of its own or it lands somewhere the user isn't looking.
|
||||||
if show_window {
|
if show_window {
|
||||||
let status = manager().status(&project_id).await;
|
let status = manager()
|
||||||
|
.status(&project_id, enabled_for(&state, &project_id))
|
||||||
|
.await;
|
||||||
if let Some(url) = status.url.as_deref() {
|
if let Some(url) = status.url.as_deref() {
|
||||||
let name = state
|
let name = state
|
||||||
.projects_store
|
.projects_store
|
||||||
@@ -311,6 +367,20 @@ pub async fn get_browser_view_match_window(project_id: String) -> Result<bool, S
|
|||||||
Ok(popout::match_window(&project_id))
|
Ok(popout::match_window(&project_id))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The project's stored browser-view opt-in.
|
||||||
|
///
|
||||||
|
/// The manager holds no copy of this — see
|
||||||
|
/// [`BrowserViewManager`](crate::browser_view::BrowserViewManager) — so every
|
||||||
|
/// status call reads it here, the way `get_auth_bridge_status` does. A project
|
||||||
|
/// that has gone away reads as off, which is the only answer that can be given
|
||||||
|
/// about a record that no longer exists.
|
||||||
|
fn enabled_for(state: &State<'_, AppState>, project_id: &str) -> bool {
|
||||||
|
state
|
||||||
|
.projects_store
|
||||||
|
.get(project_id)
|
||||||
|
.is_some_and(|p| p.browser_view_enabled)
|
||||||
|
}
|
||||||
|
|
||||||
/// The project's container, or a sentence saying why there isn't one.
|
/// The project's container, or a sentence saying why there isn't one.
|
||||||
///
|
///
|
||||||
/// Every command here needs a *running* container, and every one of them used
|
/// Every command here needs a *running* container, and every one of them used
|
||||||
@@ -344,3 +414,43 @@ async fn running_container(
|
|||||||
}
|
}
|
||||||
Ok(container_id)
|
Ok(container_id)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use std::sync::atomic::{AtomicBool, Ordering};
|
||||||
|
|
||||||
|
/// The regression: turning the view off must not leave the supervisor, the
|
||||||
|
/// proxy and the host port running just because the project record could
|
||||||
|
/// not be written — which is exactly what a missing record did.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_failed_persist_does_not_skip_the_teardown() {
|
||||||
|
let torn_down = AtomicBool::new(false);
|
||||||
|
let result = tear_down_then_report(Err("Project x not found".to_string()), async {
|
||||||
|
torn_down.store(true, Ordering::SeqCst);
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
torn_down.load(Ordering::SeqCst),
|
||||||
|
"the session must be torn down even when the store write failed"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
result.err().as_deref(),
|
||||||
|
Some("Project x not found"),
|
||||||
|
"and the write failure must still reach the caller, not be swallowed"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_successful_persist_reports_success_after_the_teardown() {
|
||||||
|
let torn_down = AtomicBool::new(false);
|
||||||
|
let result = tear_down_then_report(Ok(()), async {
|
||||||
|
torn_down.store(true, Ordering::SeqCst);
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert!(torn_down.load(Ordering::SeqCst));
|
||||||
|
assert!(result.is_ok());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -34,14 +34,22 @@
|
|||||||
//!
|
//!
|
||||||
//! ## Lifecycle
|
//! ## Lifecycle
|
||||||
//!
|
//!
|
||||||
//! Off by default and per-project opt-in, exactly like `auth_bridge_enabled`.
|
//! Off by default and per-project opt-in. The opt-in itself is
|
||||||
|
//! [`Project::browser_view_enabled`](crate::models::Project), persisted like
|
||||||
|
//! `auth_bridge_enabled` and read from the store on demand rather than cached
|
||||||
|
//! here — so the pane comes back the way it was left. What does *not* persist
|
||||||
|
//! is the session: nothing starts a viewer on app start, so a project left
|
||||||
|
//! enabled reports `enabled: true` with a state of `Off` until the pane asks
|
||||||
|
//! for one. That is deliberate, and the reason the flag and the session are
|
||||||
|
//! separate ideas — see [`BrowserViewManager::status`].
|
||||||
|
//!
|
||||||
//! One supervisor task per session owns the proxy and the viewer process, and it
|
//! One supervisor task per session owns the proxy and the viewer process, and it
|
||||||
//! is the only thing that tears them down, so every way a session can end funnels
|
//! is the only thing that tears them down, so every way a session can end funnels
|
||||||
//! through one code path:
|
//! through one code path:
|
||||||
//!
|
//!
|
||||||
//! | Trigger | Path |
|
//! | Trigger | Path |
|
||||||
//! |---|---|
|
//! |---|---|
|
||||||
//! | Turned off in the UI | `set_browser_view_enabled(false)` → [`BrowserViewManager::stop`] |
|
//! | Turned off in the UI | `set_browser_view_enabled(false)` → persist `false`, then [`BrowserViewManager::stop`] |
|
||||||
//! | Container stopped, by the UI or otherwise | supervisor's `is_container_running` check |
|
//! | Container stopped, by the UI or otherwise | supervisor's `is_container_running` check |
|
||||||
//! | Project deleted | supervisor's `store.get()` check |
|
//! | Project deleted | supervisor's `store.get()` check |
|
||||||
//! | Container rebuilt | old container stops → supervisor exits; the new one is not auto-started |
|
//! | Container rebuilt | old container stops → supervisor exits; the new one is not auto-started |
|
||||||
@@ -59,7 +67,10 @@
|
|||||||
//! orphan is reachable on container loopback only: the host-side port dies with
|
//! orphan is reachable on container loopback only: the host-side port dies with
|
||||||
//! the app, and [`crate::auth_bridge::RESERVED_CONTAINER_PORTS`] is a constant
|
//! the app, and [`crate::auth_bridge::RESERVED_CONTAINER_PORTS`] is a constant
|
||||||
//! precisely so the bridge will not mirror an orphan the next time the app
|
//! precisely so the bridge will not mirror an orphan the next time the app
|
||||||
//! starts. The next [`BrowserViewManager::start`] reclaims it.
|
//! starts. The next [`BrowserViewManager::start`] reclaims it — and since the
|
||||||
|
//! opt-in is now durable, the restarted app says `enabled` with nothing running,
|
||||||
|
//! which is exactly the state that invites the user to press the button that
|
||||||
|
//! reclaims it. Nothing reclaims it on its own, because nothing auto-starts.
|
||||||
|
|
||||||
pub mod commands;
|
pub mod commands;
|
||||||
pub mod detect;
|
pub mod detect;
|
||||||
@@ -134,7 +145,10 @@ pub enum BrowserViewState {
|
|||||||
|
|
||||||
#[derive(Debug, Clone, Serialize)]
|
#[derive(Debug, Clone, Serialize)]
|
||||||
pub struct BrowserViewStatus {
|
pub struct BrowserViewStatus {
|
||||||
/// The per-project opt-in. Off by default.
|
/// The per-project opt-in, read from the persisted project record. Off by
|
||||||
|
/// default, and true without a `Running` state whenever the view is turned
|
||||||
|
/// on but has nothing up — a stopped container, or an app that has just
|
||||||
|
/// restarted and does not auto-start viewers.
|
||||||
pub enabled: bool,
|
pub enabled: bool,
|
||||||
pub state: BrowserViewState,
|
pub state: BrowserViewState,
|
||||||
/// Fully-formed, token-bearing URL for the pane's iframe. Loopback only.
|
/// Fully-formed, token-bearing URL for the pane's iframe. Loopback only.
|
||||||
@@ -201,17 +215,20 @@ struct Session {
|
|||||||
|
|
||||||
type SessionMap = Arc<Mutex<HashMap<String, Session>>>;
|
type SessionMap = Arc<Mutex<HashMap<String, Session>>>;
|
||||||
|
|
||||||
|
/// Live sessions, and nothing else.
|
||||||
|
///
|
||||||
|
/// The per-project opt-in deliberately is **not** a field here. It lives on
|
||||||
|
/// the project record as
|
||||||
|
/// [`browser_view_enabled`](crate::models::Project::browser_view_enabled) and
|
||||||
|
/// is read from [`ProjectsStore`] at each use, exactly as
|
||||||
|
/// [`crate::auth_bridge::AuthBridgeManager`] treats `auth_bridge_enabled`:
|
||||||
|
/// one copy, durable across a restart, and impossible to get out of step with
|
||||||
|
/// what the Config tab shows. A cached copy here was the previous design and
|
||||||
|
/// its only observable behaviour was forgetting the user's choice on every
|
||||||
|
/// app start.
|
||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
pub struct BrowserViewManager {
|
pub struct BrowserViewManager {
|
||||||
sessions: SessionMap,
|
sessions: SessionMap,
|
||||||
/// The per-project opt-in.
|
|
||||||
///
|
|
||||||
/// NOTE: in memory only, so it does not survive an app restart. The durable
|
|
||||||
/// home for this is a `browser_view_enabled: bool` field on
|
|
||||||
/// `models::Project` (see the report) — `models/project.rs` is out of scope
|
|
||||||
/// for this change, so the flag lives here and the wiring is otherwise
|
|
||||||
/// identical to `auth_bridge_enabled`.
|
|
||||||
enabled: Mutex<std::collections::HashSet<String>>,
|
|
||||||
next_epoch: AtomicU64,
|
next_epoch: AtomicU64,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -226,22 +243,15 @@ pub fn manager() -> &'static Arc<BrowserViewManager> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl BrowserViewManager {
|
impl BrowserViewManager {
|
||||||
pub async fn is_enabled(&self, project_id: &str) -> bool {
|
|
||||||
self.enabled.lock().await.contains(project_id)
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn set_enabled(&self, project_id: &str, enabled: bool) {
|
|
||||||
let mut set = self.enabled.lock().await;
|
|
||||||
if enabled {
|
|
||||||
set.insert(project_id.to_string());
|
|
||||||
} else {
|
|
||||||
set.remove(project_id);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Current status without touching the container.
|
/// Current status without touching the container.
|
||||||
pub async fn status(&self, project_id: &str) -> BrowserViewStatus {
|
///
|
||||||
let enabled = self.is_enabled(project_id).await;
|
/// `enabled` is passed in rather than looked up, the way
|
||||||
|
/// [`crate::auth_bridge::AuthBridgeManager::status`] takes it: the flag is
|
||||||
|
/// the caller's to read from the store, and keeping it out of here is what
|
||||||
|
/// stops a second copy of it appearing. A project whose view is enabled but
|
||||||
|
/// whose container is stopped — or whose app has just restarted — reports
|
||||||
|
/// `enabled: true` with a state of `Off`, which is the honest answer.
|
||||||
|
pub async fn status(&self, project_id: &str, enabled: bool) -> BrowserViewStatus {
|
||||||
match self.sessions.lock().await.get(project_id) {
|
match self.sessions.lock().await.get(project_id) {
|
||||||
Some(session) => BrowserViewStatus {
|
Some(session) => BrowserViewStatus {
|
||||||
enabled,
|
enabled,
|
||||||
@@ -261,6 +271,14 @@ impl BrowserViewManager {
|
|||||||
///
|
///
|
||||||
/// Idempotent: a call while a live session exists returns that session's
|
/// Idempotent: a call while a live session exists returns that session's
|
||||||
/// status untouched, so re-opening the tab does not restart the dashboard.
|
/// status untouched, so re-opening the tab does not restart the dashboard.
|
||||||
|
///
|
||||||
|
/// This is the single funnel for turning the view **on**, so it is also
|
||||||
|
/// where the durable flag is written — both call sites (the toggle and
|
||||||
|
/// `open_page_in_container_browser`, which opens a page and then shows it)
|
||||||
|
/// mean "on", and neither can forget. The **off** direction is not
|
||||||
|
/// symmetric and must not be: [`Self::stop`] is reached by teardown paths
|
||||||
|
/// that are not the user changing their mind, so the command owns that
|
||||||
|
/// write. See [`Self::stop`].
|
||||||
pub async fn start(
|
pub async fn start(
|
||||||
&self,
|
&self,
|
||||||
project_id: String,
|
project_id: String,
|
||||||
@@ -268,7 +286,7 @@ impl BrowserViewManager {
|
|||||||
app: AppHandle,
|
app: AppHandle,
|
||||||
store: Arc<ProjectsStore>,
|
store: Arc<ProjectsStore>,
|
||||||
) -> Result<BrowserViewStatus, String> {
|
) -> Result<BrowserViewStatus, String> {
|
||||||
self.set_enabled(&project_id, true).await;
|
store.set_browser_view_enabled(&project_id, true)?;
|
||||||
|
|
||||||
// Bind the answer before acting on it: `status()` takes the same lock,
|
// Bind the answer before acting on it: `status()` takes the same lock,
|
||||||
// and this mutex is not reentrant.
|
// and this mutex is not reentrant.
|
||||||
@@ -279,7 +297,7 @@ impl BrowserViewManager {
|
|||||||
.get(&project_id)
|
.get(&project_id)
|
||||||
.is_some_and(|s| !s.supervisor.is_finished());
|
.is_some_and(|s| !s.supervisor.is_finished());
|
||||||
if already_live {
|
if already_live {
|
||||||
return Ok(self.status(&project_id).await);
|
return Ok(self.status(&project_id, true).await);
|
||||||
}
|
}
|
||||||
|
|
||||||
let detection = detect::detect(&container_id).await?;
|
let detection = detect::detect(&container_id).await?;
|
||||||
@@ -364,14 +382,21 @@ impl BrowserViewManager {
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
let status = self.status(&project_id).await;
|
let status = self.status(&project_id, true).await;
|
||||||
emit(&app, &project_id, &status);
|
emit(&app, &project_id, &status);
|
||||||
Ok(status)
|
Ok(status)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Stop one project's view and wait until its host port has been released.
|
/// Stop one project's view and wait until its host port has been released.
|
||||||
|
///
|
||||||
|
/// Tears the *session* down and deliberately leaves the durable flag alone.
|
||||||
|
/// Most callers are not the user turning the feature off — a migration
|
||||||
|
/// removes the container out from under a running view
|
||||||
|
/// (`migration_commands`), and the container can stop for any other reason
|
||||||
|
/// — and persisting `false` for those would quietly opt the project out of
|
||||||
|
/// a feature it never asked to lose. `set_browser_view_enabled(false)` is
|
||||||
|
/// the one caller that means it, and it writes the flag itself first.
|
||||||
pub async fn stop(&self, project_id: &str) {
|
pub async fn stop(&self, project_id: &str) {
|
||||||
self.set_enabled(project_id, false).await;
|
|
||||||
// Remove under the lock, then release it before awaiting: the
|
// Remove under the lock, then release it before awaiting: the
|
||||||
// supervisor takes the same lock to deregister itself on exit.
|
// supervisor takes the same lock to deregister itself on exit.
|
||||||
let session = self.sessions.lock().await.remove(project_id);
|
let session = self.sessions.lock().await.remove(project_id);
|
||||||
@@ -483,7 +508,12 @@ async fn supervise(
|
|||||||
// longer exists. The session owns it, and this is where the session ends.
|
// longer exists. The session owns it, and this is where the session ends.
|
||||||
let _ = popout::close(&app, &project_id);
|
let _ = popout::close(&app, &project_id);
|
||||||
|
|
||||||
let enabled = manager().is_enabled(&project_id).await;
|
// Straight from the store, like the auth bridge's own teardown emit: the
|
||||||
|
// session is over, but the project may well still be opted in — a stopped
|
||||||
|
// container is not a changed mind, and the pane has to show the difference.
|
||||||
|
let enabled = store
|
||||||
|
.get(&project_id)
|
||||||
|
.is_some_and(|p| p.browser_view_enabled);
|
||||||
emit(&app, &project_id, &BrowserViewStatus::off(enabled));
|
emit(&app, &project_id, &BrowserViewStatus::off(enabled));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -915,6 +945,25 @@ mod tests {
|
|||||||
assert!(s.url.is_none());
|
assert!(s.url.is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn the_opt_in_and_the_live_session_are_separate_answers() {
|
||||||
|
let manager = BrowserViewManager::default();
|
||||||
|
|
||||||
|
// Exactly what the pane reads on mount after an app restart of a
|
||||||
|
// project that was left enabled: the durable flag says on, and nothing
|
||||||
|
// auto-starts, so the state is honestly `Off`. The old in-memory flag
|
||||||
|
// could not express this — it came back `false` and the pane silently
|
||||||
|
// showed the feature as never having been turned on.
|
||||||
|
let status = manager.status("p1", true).await;
|
||||||
|
assert!(status.enabled);
|
||||||
|
assert_eq!(status.state, BrowserViewState::Off);
|
||||||
|
assert!(status.url.is_none());
|
||||||
|
|
||||||
|
// The flag belongs to the caller, read from the store. The manager
|
||||||
|
// keeps no copy, so it has nothing to contradict it with.
|
||||||
|
assert!(!manager.status("p1", false).await.enabled);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn an_unavailable_status_keeps_the_detail_the_user_needs() {
|
fn an_unavailable_status_keeps_the_detail_the_user_needs() {
|
||||||
let mut d = PlaywrightDetection::default();
|
let mut d = PlaywrightDetection::default();
|
||||||
|
|||||||
@@ -92,8 +92,111 @@ fn pick_recorded_lineage(
|
|||||||
.or_else(|| from_snapshot.filter(|v| !v.is_empty()))
|
.or_else(|| from_snapshot.filter(|v| !v.is_empty()))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Read-only. Runs two filesystem probes (~3 s each) and is therefore meant to
|
/// Reported as `probe_error` when there is genuinely nothing to read: no
|
||||||
/// be called on demand, not polled.
|
/// container, stopped or otherwise, and no snapshot image.
|
||||||
|
///
|
||||||
|
/// It used to be reported for a *stopped* container too, which was simply
|
||||||
|
/// untrue — the container was sitting right there — and it disabled Update on
|
||||||
|
/// exactly the long-lived projects that had never been recreated and so had no
|
||||||
|
/// snapshot to fall back on.
|
||||||
|
const NOTHING_TO_PROBE: &str = "This project has no container or snapshot image yet, so there is nothing to compare against the base image.";
|
||||||
|
|
||||||
|
/// Where [`get_container_staleness`] reads the project's *current* filesystem
|
||||||
|
/// from, in descending order of how current the answer is.
|
||||||
|
#[derive(Debug, PartialEq, Eq)]
|
||||||
|
enum ProbeSource {
|
||||||
|
/// `docker exec` into the live container. The only source that includes
|
||||||
|
/// everything installed since the last commit *in this session*.
|
||||||
|
RunningContainer,
|
||||||
|
/// Commit the stopped container's writable layer to a throwaway image and
|
||||||
|
/// probe that. Exactly as current as the container, which is what makes it
|
||||||
|
/// preferable to the snapshot — see below.
|
||||||
|
StoppedContainer,
|
||||||
|
/// A throwaway container from `triple-c-snapshot-<id>:latest`.
|
||||||
|
Snapshot,
|
||||||
|
/// Nothing to read: no container, no snapshot.
|
||||||
|
Nothing,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Pick the probe source. `container_running` is `None` when the project has no
|
||||||
|
/// container at all, `Some(false)` when it has a stopped one.
|
||||||
|
///
|
||||||
|
/// **A stopped container outranks the snapshot.** The snapshot image is not a
|
||||||
|
/// checkpoint — `commit_container_snapshot` runs only before a removal (a
|
||||||
|
/// config-change recreate) or inside a migration, so a project that has never
|
||||||
|
/// hit either has *no snapshot at all*, however long it has been in use, and
|
||||||
|
/// one that has is stale by everything installed since. The container's
|
||||||
|
/// writable layer is the truth in both cases. This is the same argument
|
||||||
|
/// [`mig::manifest_from_container`] already makes for the running case; it does
|
||||||
|
/// not stop applying when the container is stopped.
|
||||||
|
///
|
||||||
|
/// Getting this wrong is what made a stopped, never-recreated project report
|
||||||
|
/// "no container or snapshot image yet" — with its container sitting right
|
||||||
|
/// there — and left Update disabled on the projects that most needed it.
|
||||||
|
fn pick_probe_source(container_running: Option<bool>, snapshot_exists: bool) -> ProbeSource {
|
||||||
|
match (container_running, snapshot_exists) {
|
||||||
|
(Some(true), _) => ProbeSource::RunningContainer,
|
||||||
|
(Some(false), _) => ProbeSource::StoppedContainer,
|
||||||
|
(None, true) => ProbeSource::Snapshot,
|
||||||
|
(None, false) => ProbeSource::Nothing,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Reported as `probe_error` when another operation owns the project and there
|
||||||
|
/// is no snapshot image to read instead. Deliberately not a claim about the
|
||||||
|
/// container: nothing is wrong with it, the answer is simply not safe to take
|
||||||
|
/// right now. See [`stopped_probe_policy`].
|
||||||
|
const PROJECT_BUSY: &str = "Another operation is running on this project, so its contents could not be inspected. Try again once it finishes.";
|
||||||
|
|
||||||
|
/// What to do about a stopped container, whose probe is the expensive one: it
|
||||||
|
/// commits the writable layer before it can read anything.
|
||||||
|
#[derive(Debug, PartialEq, Eq)]
|
||||||
|
enum StoppedProbe {
|
||||||
|
/// Commit and probe. The current answer, and the default.
|
||||||
|
Commit,
|
||||||
|
/// Probe the snapshot image instead. Less current — it lags the container by
|
||||||
|
/// everything installed since the last commit — but it allocates nothing and
|
||||||
|
/// touches nothing, which is what makes it the right answer while another
|
||||||
|
/// operation owns the container.
|
||||||
|
SnapshotInstead,
|
||||||
|
/// Report rather than guess.
|
||||||
|
Defer,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Pick what to do about a stopped container.
|
||||||
|
///
|
||||||
|
/// **Never commits while the project is claimed.** `get_container_staleness`
|
||||||
|
/// takes no [`crate::project_lock`] claim of its own, by design, so a commit
|
||||||
|
/// here can overlap a Recreate or Reset — and the collision is not symmetric.
|
||||||
|
/// The probe losing is harmless: a surfaced `probe_error` the user retries. The
|
||||||
|
/// *recreate* losing is not, because `start_project_container` removes the old
|
||||||
|
/// container with a hard `?`, so a non-404 from a remove that raced this commit
|
||||||
|
/// fails the whole Start with an opaque "Failed to remove container". Reading
|
||||||
|
/// the claim costs nothing and takes that failure off the table.
|
||||||
|
fn stopped_probe_policy(project_is_busy: bool, snapshot_exists: bool) -> StoppedProbe {
|
||||||
|
match (project_is_busy, snapshot_exists) {
|
||||||
|
(false, _) => StoppedProbe::Commit,
|
||||||
|
(true, true) => StoppedProbe::SnapshotInstead,
|
||||||
|
(true, false) => StoppedProbe::Defer,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Runs two filesystem probes (~3 s each) and is therefore meant to be called
|
||||||
|
/// on demand, not polled.
|
||||||
|
///
|
||||||
|
/// **Not read-only, despite only reporting.** The stopped-container path commits
|
||||||
|
/// a throwaway image and force-removes it, which makes this a writer of a
|
||||||
|
/// `triple-c-probe-*` image and puts it in the class of thing
|
||||||
|
/// [`crate::project_lock`] exists for — and it takes no claim. That is
|
||||||
|
/// deliberate: this is what the migration banner calls to decide whether to
|
||||||
|
/// offer an update, including while a migration is in flight, so refusing it
|
||||||
|
/// under a claim would blank the banner exactly when it has the most to say.
|
||||||
|
/// The exposure is bounded to a surfaced error — a concurrent Recreate, Reset or
|
||||||
|
/// migration can remove the container out from under the commit, and the result
|
||||||
|
/// is a `probe_error` the user can retry, never a damaged container or a
|
||||||
|
/// mislabelled image. Two overlapping probes cannot collide either, because
|
||||||
|
/// probe image names are unique per call; see
|
||||||
|
/// [`crate::docker::container::get_probe_image_name`].
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
pub async fn get_container_staleness(
|
pub async fn get_container_staleness(
|
||||||
project_id: String,
|
project_id: String,
|
||||||
@@ -145,16 +248,62 @@ pub async fn get_container_staleness(
|
|||||||
};
|
};
|
||||||
|
|
||||||
// ── Probes ───────────────────────────────────────────────────────────
|
// ── Probes ───────────────────────────────────────────────────────────
|
||||||
let running = match &container_id {
|
let container_running = match &container_id {
|
||||||
Some(id) => docker::is_container_running(id).await.unwrap_or(false),
|
Some(id) => Some(docker::is_container_running(id).await.unwrap_or(false)),
|
||||||
None => false,
|
None => None,
|
||||||
};
|
};
|
||||||
let from_manifest = if running {
|
let snapshot_exists = docker::image_exists(&snapshot_image).await.unwrap_or(false);
|
||||||
mig::manifest_from_container(container_id.as_ref().unwrap()).await
|
let from_manifest = match (
|
||||||
} else if docker::image_exists(&snapshot_image).await.unwrap_or(false) {
|
pick_probe_source(container_running, snapshot_exists),
|
||||||
|
&container_id,
|
||||||
|
) {
|
||||||
|
(ProbeSource::RunningContainer, Some(id)) => mig::manifest_from_container(id).await,
|
||||||
|
(ProbeSource::StoppedContainer, Some(id)) => {
|
||||||
|
let busy = crate::project_lock::held(&project_id).is_some();
|
||||||
|
match stopped_probe_policy(busy, snapshot_exists) {
|
||||||
|
StoppedProbe::Commit => {
|
||||||
|
match mig::manifest_from_stopped_container_cached(id).await {
|
||||||
|
Ok(m) => Ok(m),
|
||||||
|
// **Never let a failed commit cost an answer the
|
||||||
|
// snapshot could have given.** Before stopped
|
||||||
|
// containers were readable at all, a stopped project
|
||||||
|
// fell straight through to its snapshot, so surfacing
|
||||||
|
// this error where the snapshot exists would make the
|
||||||
|
// banner *worse* than it was — and the ways this fails
|
||||||
|
// are the ones where the fallback matters most: a full
|
||||||
|
// disk (the commit has to allocate the whole writable
|
||||||
|
// layer; the snapshot probe allocates nothing) and a
|
||||||
|
// 409 from an operation that claimed the project after
|
||||||
|
// the check above.
|
||||||
|
Err(e) if snapshot_exists => {
|
||||||
|
log::warn!(
|
||||||
|
"Probing the stopped container for project {} failed ({}) — \
|
||||||
|
falling back to its snapshot image, which may lag it",
|
||||||
|
project_id,
|
||||||
|
e
|
||||||
|
);
|
||||||
mig::manifest_from_image(&snapshot_image).await
|
mig::manifest_from_image(&snapshot_image).await
|
||||||
} else {
|
}
|
||||||
Err("This project has no container or snapshot image yet, so there is nothing to compare against the base image.".to_string())
|
Err(e) => Err(e),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
StoppedProbe::SnapshotInstead => {
|
||||||
|
log::info!(
|
||||||
|
"Project {} is claimed by another operation — probing its snapshot image \
|
||||||
|
rather than committing the container",
|
||||||
|
project_id
|
||||||
|
);
|
||||||
|
mig::manifest_from_image(&snapshot_image).await
|
||||||
|
}
|
||||||
|
StoppedProbe::Defer => Err(PROJECT_BUSY.to_string()),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
(ProbeSource::Snapshot, _) => mig::manifest_from_image(&snapshot_image).await,
|
||||||
|
// `container_running` is `Some` exactly when `container_id` is, so the
|
||||||
|
// two arms above are the only ones those variants can reach. This arm
|
||||||
|
// is `ProbeSource::Nothing` — and now *only* that: it used to also
|
||||||
|
// swallow every stopped container, which is the bug.
|
||||||
|
(_, _) => Err(NOTHING_TO_PROBE.to_string()),
|
||||||
};
|
};
|
||||||
|
|
||||||
let (from_manifest, base_manifest) = match from_manifest {
|
let (from_manifest, base_manifest) = match from_manifest {
|
||||||
@@ -1964,6 +2113,59 @@ mod tests {
|
|||||||
assert_eq!(pick_recorded_lineage(some(""), None), None);
|
assert_eq!(pick_recorded_lineage(some(""), None), None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_stopped_container_is_probed_rather_than_reported_missing() {
|
||||||
|
// The regression: a container that exists but is stopped, with no
|
||||||
|
// snapshot ever taken, read as "nothing to compare against".
|
||||||
|
assert_eq!(
|
||||||
|
pick_probe_source(Some(false), false),
|
||||||
|
ProbeSource::StoppedContainer
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_container_outranks_the_snapshot_whether_or_not_it_is_running() {
|
||||||
|
// The snapshot lags the container by everything installed since the
|
||||||
|
// last commit, in both states.
|
||||||
|
assert_eq!(
|
||||||
|
pick_probe_source(Some(true), true),
|
||||||
|
ProbeSource::RunningContainer
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
pick_probe_source(Some(false), true),
|
||||||
|
ProbeSource::StoppedContainer
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_snapshot_is_the_fallback_only_once_the_container_is_gone() {
|
||||||
|
assert_eq!(pick_probe_source(None, true), ProbeSource::Snapshot);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn nothing_to_probe_is_reserved_for_no_container_and_no_snapshot() {
|
||||||
|
// The one case the "no container or snapshot image yet" message may
|
||||||
|
// still describe.
|
||||||
|
assert_eq!(pick_probe_source(None, false), ProbeSource::Nothing);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_stopped_container_is_committed_only_when_nothing_else_owns_the_project() {
|
||||||
|
assert_eq!(stopped_probe_policy(false, false), StoppedProbe::Commit);
|
||||||
|
assert_eq!(stopped_probe_policy(false, true), StoppedProbe::Commit);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_busy_project_falls_back_rather_than_racing_a_recreate() {
|
||||||
|
// The snapshot lags, but a stale answer beats failing someone's Start.
|
||||||
|
assert_eq!(
|
||||||
|
stopped_probe_policy(true, true),
|
||||||
|
StoppedProbe::SnapshotInstead
|
||||||
|
);
|
||||||
|
// Nothing to fall back to: say so instead of committing anyway.
|
||||||
|
assert_eq!(stopped_probe_policy(true, false), StoppedProbe::Defer);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn byte_sizes_read_the_way_a_disk_warning_should() {
|
fn byte_sizes_read_the_way_a_disk_warning_should() {
|
||||||
assert_eq!(human_bytes(512), "512 B");
|
assert_eq!(human_bytes(512), "512 B");
|
||||||
|
|||||||
@@ -1036,7 +1036,6 @@ fn pending_cleanup_is_stale(recorded_at: &str, now: chrono::DateTime<chrono::Utc
|
|||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
pub async fn update_project(
|
pub async fn update_project(
|
||||||
project: serde_json::Value,
|
project: serde_json::Value,
|
||||||
app_handle: tauri::AppHandle,
|
|
||||||
state: State<'_, AppState>,
|
state: State<'_, AppState>,
|
||||||
) -> Result<Project, String> {
|
) -> Result<Project, String> {
|
||||||
// Taken as raw JSON, then deserialised, for one reason: a secret field that
|
// Taken as raw JSON, then deserialised, for one reason: a secret field that
|
||||||
@@ -1098,37 +1097,57 @@ pub async fn update_project(
|
|||||||
// [`crate::models::validate_env_vars_update`].
|
// [`crate::models::validate_env_vars_update`].
|
||||||
crate::models::validate_env_vars_update(&stored.custom_env_vars, &project.custom_env_vars)?;
|
crate::models::validate_env_vars_update(&stored.custom_env_vars, &project.custom_env_vars)?;
|
||||||
|
|
||||||
project.container_id = stored.container_id;
|
restore_store_owned_fields(&mut project, &stored);
|
||||||
project.status = stored.status;
|
|
||||||
project.created_at = stored.created_at;
|
|
||||||
project.updated_at = chrono::Utc::now().to_rfc3339();
|
project.updated_at = chrono::Utc::now().to_rfc3339();
|
||||||
|
|
||||||
store_secrets_for_project(&project, &explicitly_cleared)?;
|
store_secrets_for_project(&project, &explicitly_cleared)?;
|
||||||
let updated = state.projects_store.update(project)?;
|
|
||||||
|
|
||||||
// `auth_bridge_enabled` can arrive through this generic save as well as
|
// Nothing reconciles the *running* auth bridge here any more, and there is
|
||||||
// through `set_auth_bridge_enabled`, so reconcile the running bridge with
|
// nothing left for such a step to do. This command can no longer change
|
||||||
// whatever was just persisted. `start` is idempotent and `stop` is a no-op
|
// `auth_bridge_enabled` at all (see [`restore_store_owned_fields`]), so a
|
||||||
// when nothing is running, so this is safe on every project save.
|
// reconcile could only ever re-assert what was already true. The paths that
|
||||||
if updated.auth_bridge_enabled {
|
// do change it each own their own side effect: `set_auth_bridge_enabled`
|
||||||
if let Some(ref container_id) = updated.container_id {
|
// starts or stops the bridge itself, [`start_project_container`] arms it
|
||||||
if docker::is_container_running(container_id).await.unwrap_or(false) {
|
// when the container comes up, and `reconcile_project_statuses` re-arms it
|
||||||
state
|
// for every already-running container at launch. The version of this that
|
||||||
.auth_bridge
|
// re-asserted on every save is what turned a stale flag in a payload into a
|
||||||
.start(
|
// restarted bridge.
|
||||||
updated.id.clone(),
|
state.projects_store.update(project)
|
||||||
container_id.clone(),
|
|
||||||
app_handle,
|
|
||||||
state.projects_store.clone(),
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
state.auth_bridge.stop(&updated.id).await;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(updated)
|
/// Restore onto `project` the fields whose value belongs to the store rather
|
||||||
|
/// than to whoever is saving the project. See the comment above `stored` in
|
||||||
|
/// [`update_project`] for `container_id`, `status` and `created_at`.
|
||||||
|
///
|
||||||
|
/// **Both feature flags are in here, for one reason that covers them equally:
|
||||||
|
/// neither ever arrives through this command as an edit.** Each has a
|
||||||
|
/// dedicated setter — [`crate::browser_view::commands::set_browser_view_enabled`]
|
||||||
|
/// and [`crate::commands::auth_bridge_commands::set_auth_bridge_enabled`] —
|
||||||
|
/// and that setter is the only control the UI offers for it. Neither is wired
|
||||||
|
/// into the Config tab's `save`: the browser view's toggle lives in the Browser
|
||||||
|
/// tab, and `AuthBridgeRow`'s switch calls `set_auth_bridge_enabled` directly
|
||||||
|
/// even though it is rendered *in* the Config tab, because that tab's editors
|
||||||
|
/// are disabled while the container runs and the bridge is precisely the thing
|
||||||
|
/// a user needs to flip while a login is hanging.
|
||||||
|
///
|
||||||
|
/// So the flags in an incoming payload are never a choice — they are whatever
|
||||||
|
/// the frontend was told when it loaded the project, and the setters do not
|
||||||
|
/// write their new value back into frontend app state. Every unrelated save
|
||||||
|
/// (a renamed session, an env var, a mount name) carries that snapshot back.
|
||||||
|
/// Taking it would silently undo a toggle made since.
|
||||||
|
///
|
||||||
|
/// This restored only `browser_view_enabled` before, on the stated belief that
|
||||||
|
/// the Config tab edited `auth_bridge_enabled` through this save. It does not.
|
||||||
|
/// The consequence was specific: a user turns the bridge off — having been told
|
||||||
|
/// a bridged port is unauthenticated and reachable by any local process — then
|
||||||
|
/// closes a renamed terminal tab, and the stale `true` in that save re-persisted
|
||||||
|
/// and restarted the bridge.
|
||||||
|
fn restore_store_owned_fields(project: &mut Project, stored: &Project) {
|
||||||
|
project.container_id = stored.container_id.clone();
|
||||||
|
project.status = stored.status.clone();
|
||||||
|
project.browser_view_enabled = stored.browser_view_enabled;
|
||||||
|
project.auth_bridge_enabled = stored.auth_bridge_enabled;
|
||||||
|
project.created_at = stored.created_at.clone();
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
@@ -2186,4 +2205,89 @@ mod tests {
|
|||||||
// Changing it to a different root is a change, and refused.
|
// Changing it to a different root is a change, and refused.
|
||||||
assert!(validate_mounted_host_path("x", Some("/"), Some("C:\\")).is_err());
|
assert!(validate_mounted_host_path("x", Some("/"), Some("C:\\")).is_err());
|
||||||
}
|
}
|
||||||
|
// ── Fields a generic save does not get to write ───────────────────────
|
||||||
|
|
||||||
|
/// A project as the store holds it, plus the copy the frontend is about to
|
||||||
|
/// save back: same record, one unrelated edit, and the flags as they were
|
||||||
|
/// when the frontend last loaded it.
|
||||||
|
fn stored_and_stale_payload() -> (Project, Project) {
|
||||||
|
let mut stored = Project::new("demo".to_string(), Vec::new());
|
||||||
|
stored.container_id = Some("abc123".to_string());
|
||||||
|
stored.status = ProjectStatus::Running;
|
||||||
|
|
||||||
|
let mut payload = stored.clone();
|
||||||
|
payload.container_id = None;
|
||||||
|
payload.status = ProjectStatus::Stopped;
|
||||||
|
payload
|
||||||
|
.renamed_session_names
|
||||||
|
.insert("s1".to_string(), "build".to_string());
|
||||||
|
|
||||||
|
(stored, payload)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The regression. The user turns the auth bridge off — the switch calls
|
||||||
|
/// `set_auth_bridge_enabled`, which persists `false` and stops the bridge,
|
||||||
|
/// and writes nothing back into the frontend's copy of the project. Every
|
||||||
|
/// holder of that copy still has `auth_bridge_enabled: true`, and the next
|
||||||
|
/// unrelated save (closing a renamed terminal tab) posts it back. That save
|
||||||
|
/// must not re-enable the bridge.
|
||||||
|
#[test]
|
||||||
|
fn a_stale_auth_bridge_flag_in_a_save_cannot_re_enable_a_disabled_bridge() {
|
||||||
|
let (mut stored, mut payload) = stored_and_stale_payload();
|
||||||
|
stored.auth_bridge_enabled = false;
|
||||||
|
payload.auth_bridge_enabled = true;
|
||||||
|
|
||||||
|
restore_store_owned_fields(&mut payload, &stored);
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
!payload.auth_bridge_enabled,
|
||||||
|
"a save must not be able to turn the bridge back on: the stored value is the user's"
|
||||||
|
);
|
||||||
|
// The edit the save was actually for still goes through.
|
||||||
|
assert_eq!(
|
||||||
|
payload.renamed_session_names.get("s1").map(String::as_str),
|
||||||
|
Some("build")
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The mirror image, and the reason the serde default going to `true`
|
||||||
|
/// made this worse: a pre-existing record with no `auth_bridge_enabled`
|
||||||
|
/// key reads as enabled, so the stale payload is `true` for every project
|
||||||
|
/// that predates the field. A user who has *not* turned the bridge off is
|
||||||
|
/// equally entitled to have the store's answer win.
|
||||||
|
#[test]
|
||||||
|
fn an_enabled_bridge_is_left_enabled_by_the_same_rule() {
|
||||||
|
let (mut stored, mut payload) = stored_and_stale_payload();
|
||||||
|
stored.auth_bridge_enabled = true;
|
||||||
|
payload.auth_bridge_enabled = false;
|
||||||
|
|
||||||
|
restore_store_owned_fields(&mut payload, &stored);
|
||||||
|
|
||||||
|
assert!(payload.auth_bridge_enabled);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The flag that was already restored, kept under test beside the one that
|
||||||
|
/// was not — the two are owned by their setters for the same reason and
|
||||||
|
/// must not drift apart again.
|
||||||
|
#[test]
|
||||||
|
fn a_stale_browser_view_flag_cannot_undo_the_panes_toggle_either() {
|
||||||
|
let (mut stored, mut payload) = stored_and_stale_payload();
|
||||||
|
stored.browser_view_enabled = true;
|
||||||
|
payload.browser_view_enabled = false;
|
||||||
|
|
||||||
|
restore_store_owned_fields(&mut payload, &stored);
|
||||||
|
|
||||||
|
assert!(payload.browser_view_enabled);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_container_handle_status_and_creation_time_still_come_from_the_store() {
|
||||||
|
let (stored, mut payload) = stored_and_stale_payload();
|
||||||
|
|
||||||
|
restore_store_owned_fields(&mut payload, &stored);
|
||||||
|
|
||||||
|
assert_eq!(payload.container_id.as_deref(), Some("abc123"));
|
||||||
|
assert_eq!(payload.status, ProjectStatus::Running);
|
||||||
|
assert_eq!(payload.created_at, stored.created_at);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3052,6 +3052,118 @@ fn blanked_secret_env() -> Vec<String> {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Image-name prefix for the throwaway commit a staleness probe of a stopped
|
||||||
|
/// container makes. The reaper's only handle on a leftover — see
|
||||||
|
/// [`crate::docker::migration::reap_probe_images`] — so nothing else may use it.
|
||||||
|
pub const PROBE_IMAGE_PREFIX: &str = "triple-c-probe-";
|
||||||
|
|
||||||
|
/// The throwaway image a staleness probe of a **stopped** container commits to.
|
||||||
|
///
|
||||||
|
/// **Unique per call**, and both halves of the name earn their place: the
|
||||||
|
/// container id prefix makes a leftover traceable in `docker images`, and the
|
||||||
|
/// counter makes two overlapping probes independent.
|
||||||
|
///
|
||||||
|
/// An earlier version of this was deliberately *stable* per container, on the
|
||||||
|
/// theory that the next probe would move the tag off an abandoned image and
|
||||||
|
/// leave it dangling for [`sweep_orphaned_snapshots`]. That was wrong twice
|
||||||
|
/// over. A container id does not survive a recreate, so for most leftovers
|
||||||
|
/// there is no "next probe of the same container" and the image was stranded
|
||||||
|
/// permanently; and a stable name made two concurrent probes fight over one
|
||||||
|
/// tag, where whichever finished first force-removed the image the other was
|
||||||
|
/// still reading and turned a healthy project into a bogus `probe_error`.
|
||||||
|
/// Uniqueness fixes both, and [`crate::docker::migration::reap_probe_images`]
|
||||||
|
/// is what collects the leftovers instead.
|
||||||
|
pub fn get_probe_image_name(container_id: &str) -> String {
|
||||||
|
use std::sync::atomic::{AtomicU64, Ordering};
|
||||||
|
static SEQ: AtomicU64 = AtomicU64::new(0);
|
||||||
|
|
||||||
|
let short: String = container_id.chars().take(12).collect();
|
||||||
|
let nanos = std::time::SystemTime::now()
|
||||||
|
.duration_since(std::time::UNIX_EPOCH)
|
||||||
|
.map(|d| d.as_nanos())
|
||||||
|
.unwrap_or(0);
|
||||||
|
format!(
|
||||||
|
"{}{}-{}-{}:latest",
|
||||||
|
PROBE_IMAGE_PREFIX,
|
||||||
|
short,
|
||||||
|
nanos,
|
||||||
|
SEQ.fetch_add(1, Ordering::Relaxed)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Commit a **stopped** container's filesystem to a throwaway image, returning
|
||||||
|
/// its name. The caller owns the image and must remove it.
|
||||||
|
///
|
||||||
|
/// This exists so a stopped project can be read at all. `docker exec` needs a
|
||||||
|
/// running container and the snapshot image is not a checkpoint — see
|
||||||
|
/// [`crate::commands::migration_commands`]'s probe-source pick — so without
|
||||||
|
/// this there is no way to see inside a project that is merely stopped.
|
||||||
|
///
|
||||||
|
/// ## Why it is tagged at all
|
||||||
|
///
|
||||||
|
/// An untagged commit would be tidier: untagged plus the `triple-c.managed=true`
|
||||||
|
/// that `docker commit` copies off the container is exactly the pair
|
||||||
|
/// [`sweep_orphaned_snapshots`] already collects, so a leftover would self-heal
|
||||||
|
/// with no new machinery. **It is not available.** `bollard`'s `Commit` response
|
||||||
|
/// model deserialises `"ID"` while the daemon sends `"Id"`, so
|
||||||
|
/// `commit_container` hands back `id: None` every time and there is no
|
||||||
|
/// reference left to probe. Neither existing commit site notices, because both
|
||||||
|
/// discard the response. Verified against Engine 29.6, bollard 0.18.1.
|
||||||
|
///
|
||||||
|
/// So the image needs a name, a tagged image is not dangling, and the sweep
|
||||||
|
/// therefore cannot be the safety net. [`crate::docker::migration::reap_probe_images`]
|
||||||
|
/// is, and [`get_probe_image_name`] carries the rest of that argument.
|
||||||
|
///
|
||||||
|
/// ## What is in the image, and what is not
|
||||||
|
///
|
||||||
|
/// `pause: false` because nothing is running — pausing a stopped container is
|
||||||
|
/// an error, the same reason [`recommit_without_secrets`]'s scratch commit
|
||||||
|
/// passes `false`.
|
||||||
|
///
|
||||||
|
/// Secrets are blanked from the env for the same reason
|
||||||
|
/// [`commit_container_snapshot`] blanks them: the commit bakes the container's
|
||||||
|
/// full ENV into the image, and "it only lives a few seconds" is not a property
|
||||||
|
/// this function can promise after a crash.
|
||||||
|
///
|
||||||
|
/// **The writable layer is committed unscrubbed, and that is unavoidable here.**
|
||||||
|
/// [`commit_container_snapshot`] runs [`scrub_writable_layer`] first precisely
|
||||||
|
/// because a commit stacks a layer and never rewrites one — but that scrub is a
|
||||||
|
/// `docker exec`, which is exactly what a stopped container cannot serve, and
|
||||||
|
/// scrubbing is not wanted anyway: the probe's whole job is to report the
|
||||||
|
/// filesystem as it actually is. What makes it acceptable is that this copies
|
||||||
|
/// bytes that are *already on this disk* in the container's own writable layer,
|
||||||
|
/// into an image that is never pushed, never created from, and reaped — so it
|
||||||
|
/// duplicates data inside one trust domain rather than widening it. That
|
||||||
|
/// argument depends on the reaping actually happening; treat
|
||||||
|
/// [`crate::docker::migration::reap_probe_images`] as load-bearing, not tidying.
|
||||||
|
pub async fn commit_container_for_probe(container_id: &str) -> Result<String, String> {
|
||||||
|
let docker = get_docker()?;
|
||||||
|
let image_name = get_probe_image_name(container_id);
|
||||||
|
let (repo, tag) = image_name
|
||||||
|
.rsplit_once(':')
|
||||||
|
.map(|(r, t)| (r.to_string(), t.to_string()))
|
||||||
|
.expect("get_probe_image_name always emits a tag");
|
||||||
|
|
||||||
|
docker
|
||||||
|
.commit_container(
|
||||||
|
CommitContainerOptions {
|
||||||
|
container: container_id.to_string(),
|
||||||
|
repo,
|
||||||
|
tag,
|
||||||
|
pause: false,
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
Config::<String> {
|
||||||
|
env: Some(blanked_secret_env()),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.map_err(|e| format!("Failed to commit stopped container {}: {}", container_id, e))?;
|
||||||
|
|
||||||
|
Ok(image_name)
|
||||||
|
}
|
||||||
|
|
||||||
/// Whether `env` (an image's `Config.Env`) holds a non-empty value for any
|
/// Whether `env` (an image's `Config.Env`) holds a non-empty value for any
|
||||||
/// name in [`SECRET_ENV_KEYS`].
|
/// name in [`SECRET_ENV_KEYS`].
|
||||||
fn env_holds_a_secret(env: &[String]) -> bool {
|
fn env_holds_a_secret(env: &[String]) -> bool {
|
||||||
@@ -3518,9 +3630,10 @@ pub async fn remove_snapshot_image(project: &Project) -> Result<(), String> {
|
|||||||
remove_image_by_name(&get_snapshot_image_name(project)).await
|
remove_image_by_name(&get_snapshot_image_name(project)).await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Remove a Docker image by name/tag, treating "does not exist" as success.
|
/// Remove a Docker image by name, tag or **id**, treating "does not exist" as
|
||||||
/// Shared by [`remove_snapshot_image`] and the pending-cleanup retry, which
|
/// success. Shared by [`remove_snapshot_image`], the pending-cleanup retry
|
||||||
/// only has the image name (the project record is already gone by then).
|
/// (which only has the image name — the project record is already gone by
|
||||||
|
/// then), and the staleness probe's throwaway commit, which has only an id.
|
||||||
pub async fn remove_image_by_name(image_name: &str) -> Result<(), String> {
|
pub async fn remove_image_by_name(image_name: &str) -> Result<(), String> {
|
||||||
let docker = get_docker()?;
|
let docker = get_docker()?;
|
||||||
|
|
||||||
@@ -3536,7 +3649,7 @@ pub async fn remove_image_by_name(image_name: &str) -> Result<(), String> {
|
|||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(_) => {
|
Ok(_) => {
|
||||||
log::info!("Removed snapshot image {}", image_name);
|
log::info!("Removed image {}", image_name);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
Err(bollard::errors::Error::DockerResponseServerError {
|
Err(bollard::errors::Error::DockerResponseServerError {
|
||||||
@@ -4464,6 +4577,29 @@ mod tests {
|
|||||||
assert!(env_holds_a_secret(&env));
|
assert!(env_holds_a_secret(&env));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The probe image's name must be **unique per call**. A stable name was
|
||||||
|
/// tried and is wrong twice over: a container id does not survive a
|
||||||
|
/// recreate, so a crashed probe's leftover would never be reclaimed by "the
|
||||||
|
/// next probe of the same container"; and two concurrent probes sharing one
|
||||||
|
/// tag means whichever finishes first force-removes the image the other is
|
||||||
|
/// still reading. See `commit_container_for_probe` and `reap_probe_images`.
|
||||||
|
#[test]
|
||||||
|
fn probe_image_names_are_unique_per_call_and_reapable_by_prefix() {
|
||||||
|
let id = "75993e6d5e1ab473b029a408c5ff0339";
|
||||||
|
let a = get_probe_image_name(id);
|
||||||
|
let b = get_probe_image_name(id);
|
||||||
|
assert_ne!(a, b, "two probes of one container must not share a tag");
|
||||||
|
|
||||||
|
// The prefix is the reaper's only handle on a leftover, so every name
|
||||||
|
// has to carry it — and it must not be the snapshot namespace, which is
|
||||||
|
// what a project is rebuilt from.
|
||||||
|
assert!(a.starts_with(PROBE_IMAGE_PREFIX), "{}", a);
|
||||||
|
assert!(!a.starts_with("triple-c-snapshot-"), "{}", a);
|
||||||
|
// Traceable back to its container, which is the point of the prefix.
|
||||||
|
assert!(a.contains("75993e6d5e1a"), "{}", a);
|
||||||
|
assert!(a.ends_with(":latest"), "{}", a);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn the_scrub_report_only_claims_success_when_nothing_is_left() {
|
fn the_scrub_report_only_claims_success_when_nothing_is_left() {
|
||||||
let clean = SnapshotScrubReport {
|
let clean = SnapshotScrubReport {
|
||||||
|
|||||||
@@ -886,6 +886,100 @@ pub async fn reap_probe_containers() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Remove throwaway images left behind by a staleness probe of a stopped
|
||||||
|
/// container — [`super::container::commit_container_for_probe`]'s commits.
|
||||||
|
///
|
||||||
|
/// **Load-bearing, not tidying.** A probe image is *tagged*, because bollard
|
||||||
|
/// gives no image id back from a commit and there has to be something to probe.
|
||||||
|
/// Tagged means not dangling, so [`super::container::sweep_orphaned_snapshots`]
|
||||||
|
/// — which collects every other kind of orphan this app can leave — will never
|
||||||
|
/// see one. Without this, a probe that dies between its commit and its own
|
||||||
|
/// cleanup (SIGKILL, a crash, a 409 from a concurrent remove) strands a
|
||||||
|
/// multi-gigabyte image that **no code path can ever reclaim**, and there is no
|
||||||
|
/// UI to find it either. That is the one leak in this app with no floor on it,
|
||||||
|
/// so this runs at startup beside [`reap_probe_containers`].
|
||||||
|
///
|
||||||
|
/// Age-gated for exactly the reason that one is: `reference=` is a daemon-wide
|
||||||
|
/// filter, so a second copy of the app probing a project on the same daemon has
|
||||||
|
/// images matching this glob, and removing one mid-capture fails that probe with
|
||||||
|
/// "No such image" — the bogus `probe_error` the staleness work exists to get
|
||||||
|
/// rid of. In-process state cannot see the other instance, so age is the only
|
||||||
|
/// brake, and [`PROBE_REAP_MIN_AGE_SECS`] is already the right one: a probe is a
|
||||||
|
/// `find` over a root filesystem, not a multi-minute job.
|
||||||
|
///
|
||||||
|
/// Never fails the caller. Housekeeping, like every other sweep here.
|
||||||
|
pub async fn reap_probe_images() {
|
||||||
|
use bollard::image::{ListImagesOptions, RemoveImageOptions};
|
||||||
|
|
||||||
|
let docker = match get_docker() {
|
||||||
|
Ok(d) => d,
|
||||||
|
Err(e) => {
|
||||||
|
log::warn!("Could not reap leftover probe images: {}", e);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
let filters = HashMap::from([(
|
||||||
|
"reference".to_string(),
|
||||||
|
vec![format!("{}*", super::container::PROBE_IMAGE_PREFIX)],
|
||||||
|
)]);
|
||||||
|
let images = match docker
|
||||||
|
.list_images(Some(ListImagesOptions {
|
||||||
|
all: false,
|
||||||
|
filters,
|
||||||
|
..Default::default()
|
||||||
|
}))
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
Ok(images) => images,
|
||||||
|
Err(e) => {
|
||||||
|
log::warn!("Could not list leftover probe images: {}", e);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
let now = chrono::Utc::now().timestamp();
|
||||||
|
for image in images {
|
||||||
|
// Unlike a container summary, an image summary always carries a
|
||||||
|
// `Created`, so there is no unknown-age case to defend against here.
|
||||||
|
if now - image.created < PROBE_REAP_MIN_AGE_SECS {
|
||||||
|
log::info!(
|
||||||
|
"Leaving probe image {:?} alone — it is younger than {} minutes, so it may belong \
|
||||||
|
to another Triple-C instance's live probe",
|
||||||
|
image.repo_tags,
|
||||||
|
PROBE_REAP_MIN_AGE_SECS / 60
|
||||||
|
);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
// By **tag**, never by image id. A `force` removal by id untags an
|
||||||
|
// image everywhere, so an id that happens to carry another name loses
|
||||||
|
// that name too — which is how a test fixture that tagged
|
||||||
|
// `alpine:latest` into this namespace deleted the user's alpine. A real
|
||||||
|
// leftover has exactly the one probe tag, so removing the tag removes
|
||||||
|
// the image; anything else keeps whatever other names it has.
|
||||||
|
for tag in image
|
||||||
|
.repo_tags
|
||||||
|
.iter()
|
||||||
|
.filter(|t| t.starts_with(super::container::PROBE_IMAGE_PREFIX))
|
||||||
|
{
|
||||||
|
log::info!("Removing leftover probe image {}", tag);
|
||||||
|
if let Err(e) = docker
|
||||||
|
.remove_image(
|
||||||
|
tag,
|
||||||
|
Some(RemoveImageOptions {
|
||||||
|
force: true,
|
||||||
|
noprune: false,
|
||||||
|
}),
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
log::warn!("Could not remove leftover probe image {}: {}", tag, e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// How old a `triple-c.probe=migration` container must be before
|
/// How old a `triple-c.probe=migration` container must be before
|
||||||
/// [`reap_probe_containers`] will force-remove it, in seconds.
|
/// [`reap_probe_containers`] will force-remove it, in seconds.
|
||||||
///
|
///
|
||||||
@@ -993,6 +1087,119 @@ pub async fn manifest_from_container(container_id: &str) -> Result<Manifest, Str
|
|||||||
Ok(parse_manifest(&out))
|
Ok(parse_manifest(&out))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Cached stopped-container manifests, keyed by container id, each paired with
|
||||||
|
/// the container's `FinishedAt` at the time it was captured.
|
||||||
|
///
|
||||||
|
/// **Sound because a stopped container's writable layer cannot change.** Nothing
|
||||||
|
/// can write to it while it is not running, so a manifest captured after it
|
||||||
|
/// stopped stays true until it is started again — and `FinishedAt` moves on
|
||||||
|
/// every stop, which is what makes the key exact rather than merely plausible.
|
||||||
|
///
|
||||||
|
/// This exists because `get_container_staleness` is called from a `useEffect`
|
||||||
|
/// that fires whenever the container settles, so simply opening a stopped
|
||||||
|
/// project's Overview probes it. Uncached that meant a `docker commit` of the
|
||||||
|
/// whole writable layer per visit — measured at 44 s on a real project — where
|
||||||
|
/// before this feature the same visit cost one throwaway container or nothing at
|
||||||
|
/// all. A regression like that is not worth the answer it buys.
|
||||||
|
///
|
||||||
|
/// Capped, because a `Manifest` of a real container is a few MB: this only has
|
||||||
|
/// to serve "the project whose page is open", so a handful of entries is the
|
||||||
|
/// whole working set and the oldest is dropped past that.
|
||||||
|
static STOPPED_MANIFEST_CACHE: std::sync::Mutex<
|
||||||
|
Option<Vec<(String, String, Manifest)>>,
|
||||||
|
> = std::sync::Mutex::new(None);
|
||||||
|
|
||||||
|
/// How many stopped-container manifests [`STOPPED_MANIFEST_CACHE`] keeps.
|
||||||
|
const STOPPED_MANIFEST_CACHE_MAX: usize = 4;
|
||||||
|
|
||||||
|
/// `FinishedAt` for a container, the cache's validity token. `None` when it
|
||||||
|
/// cannot be read, which is never treated as a hit.
|
||||||
|
async fn container_finished_at(container_id: &str) -> Option<String> {
|
||||||
|
let docker = get_docker().ok()?;
|
||||||
|
docker
|
||||||
|
.inspect_container(container_id, None)
|
||||||
|
.await
|
||||||
|
.ok()?
|
||||||
|
.state?
|
||||||
|
.finished_at
|
||||||
|
.filter(|s| !s.is_empty())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Capture a [`Manifest`] from a **stopped** container, reusing a cached one
|
||||||
|
/// when the container has not been started since it was taken.
|
||||||
|
///
|
||||||
|
/// See [`STOPPED_MANIFEST_CACHE`] for why this is exact and why it is needed.
|
||||||
|
pub async fn manifest_from_stopped_container_cached(
|
||||||
|
container_id: &str,
|
||||||
|
) -> Result<Manifest, String> {
|
||||||
|
let finished_at = container_finished_at(container_id).await;
|
||||||
|
|
||||||
|
if let Some(token) = &finished_at {
|
||||||
|
let guard = STOPPED_MANIFEST_CACHE.lock();
|
||||||
|
if let Ok(cache) = guard {
|
||||||
|
if let Some(entries) = cache.as_ref() {
|
||||||
|
if let Some((_, _, manifest)) = entries
|
||||||
|
.iter()
|
||||||
|
.find(|(id, tok, _)| id == container_id && tok == token)
|
||||||
|
{
|
||||||
|
log::debug!(
|
||||||
|
"Reusing the cached manifest for stopped container {}",
|
||||||
|
container_id
|
||||||
|
);
|
||||||
|
return Ok(manifest.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
let manifest = manifest_from_stopped_container(container_id).await?;
|
||||||
|
|
||||||
|
// Only cacheable if the container's state could be read at all; an unknown
|
||||||
|
// `FinishedAt` means there is no token that could later be compared.
|
||||||
|
if let Some(token) = finished_at {
|
||||||
|
if let Ok(mut cache) = STOPPED_MANIFEST_CACHE.lock() {
|
||||||
|
let entries = cache.get_or_insert_with(Vec::new);
|
||||||
|
entries.retain(|(id, _, _)| id != container_id);
|
||||||
|
entries.push((container_id.to_string(), token, manifest.clone()));
|
||||||
|
while entries.len() > STOPPED_MANIFEST_CACHE_MAX {
|
||||||
|
entries.remove(0);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(manifest)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Capture a [`Manifest`] from a **stopped** container.
|
||||||
|
///
|
||||||
|
/// Commits the container's writable layer to a throwaway image, probes that,
|
||||||
|
/// and removes it. This is as current as [`manifest_from_container`] — it reads
|
||||||
|
/// the same filesystem — and it is why a stopped project no longer has to fall
|
||||||
|
/// back to its snapshot image, which may not exist at all and lags the
|
||||||
|
/// container by everything installed since the last commit when it does.
|
||||||
|
///
|
||||||
|
/// The image is removed on every path, including a failed probe. See
|
||||||
|
/// [`super::container::commit_container_for_probe`] for what a crash in the
|
||||||
|
/// window between the two costs, and why it is bounded.
|
||||||
|
pub async fn manifest_from_stopped_container(container_id: &str) -> Result<Manifest, String> {
|
||||||
|
let image = super::container::commit_container_for_probe(container_id).await?;
|
||||||
|
|
||||||
|
let manifest = manifest_from_image(&image)
|
||||||
|
.await
|
||||||
|
.map_err(|e| format!("Probe of the stopped container did not complete: {}", e));
|
||||||
|
|
||||||
|
if let Err(e) = super::container::remove_image_by_name(&image).await {
|
||||||
|
log::warn!(
|
||||||
|
"Could not remove the staleness probe's throwaway image {}: {} — `reap_probe_images` \
|
||||||
|
collects it at the next app start; the orphan sweep never will, because it is tagged",
|
||||||
|
image,
|
||||||
|
e
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
manifest
|
||||||
|
}
|
||||||
|
|
||||||
/// The image ID (`sha256:…`) of a local image, or `None` if it is not present.
|
/// The image ID (`sha256:…`) of a local image, or `None` if it is not present.
|
||||||
///
|
///
|
||||||
/// Deliberately the **ID**, not a repo digest: locally built images and custom
|
/// Deliberately the **ID**, not a repo digest: locally built images and custom
|
||||||
@@ -2146,4 +2353,258 @@ mod tests {
|
|||||||
assert!(!pin_is_reapable("pre-migration-handmade", false, ancient, &now));
|
assert!(!pin_is_reapable("pre-migration-handmade", false, ancient, &now));
|
||||||
assert!(!pin_is_reapable("latest", false, ancient, &now));
|
assert!(!pin_is_reapable("latest", false, ancient, &now));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── Live Docker ─────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
/// The cache serves a second read of an unchanged stopped container, and —
|
||||||
|
/// the half that matters — stops serving it the moment the container is
|
||||||
|
/// started and stopped again. If invalidation were wrong this would report a
|
||||||
|
/// filesystem the project no longer has, and a migration would be planned
|
||||||
|
/// against it.
|
||||||
|
///
|
||||||
|
/// ```text
|
||||||
|
/// cargo test -- --ignored --nocapture stopped_manifest_cache
|
||||||
|
/// ```
|
||||||
|
#[cfg(unix)]
|
||||||
|
#[tokio::test]
|
||||||
|
#[ignore = "needs a Docker daemon; creates, commits and removes a throwaway container"]
|
||||||
|
async fn the_stopped_manifest_cache_survives_a_reread_but_not_a_restart() {
|
||||||
|
fn docker_cli(args: &[&str]) -> String {
|
||||||
|
let out = std::process::Command::new("docker")
|
||||||
|
.args(args)
|
||||||
|
.output()
|
||||||
|
.expect("docker CLI");
|
||||||
|
assert!(
|
||||||
|
out.status.success(),
|
||||||
|
"docker {:?} failed: {}",
|
||||||
|
args,
|
||||||
|
String::from_utf8_lossy(&out.stderr)
|
||||||
|
);
|
||||||
|
String::from_utf8_lossy(&out.stdout).trim().to_string()
|
||||||
|
}
|
||||||
|
|
||||||
|
let image = std::env::var("TRIPLE_C_TEST_IMAGE")
|
||||||
|
.unwrap_or_else(|_| "ghcr.io/shadowdao/triple-c-sandbox:latest".to_string());
|
||||||
|
let first = format!("/opt/cache-marker-a-{}", std::process::id());
|
||||||
|
let second = format!("/opt/cache-marker-b-{}", std::process::id());
|
||||||
|
|
||||||
|
let id = docker_cli(&[
|
||||||
|
"run", "-d", "--label", "triple-c.managed=true",
|
||||||
|
"--entrypoint", "/bin/sh",
|
||||||
|
&image, "-c", "sleep 600",
|
||||||
|
]);
|
||||||
|
let cleanup = || {
|
||||||
|
let _ = std::process::Command::new("docker")
|
||||||
|
.args(["rm", "-f", &id])
|
||||||
|
.output();
|
||||||
|
};
|
||||||
|
|
||||||
|
docker_cli(&["exec", &id, "mkdir", "-p", &first]);
|
||||||
|
docker_cli(&["stop", "-t", "1", &id]);
|
||||||
|
|
||||||
|
let t0 = std::time::Instant::now();
|
||||||
|
let cold = manifest_from_stopped_container_cached(&id).await;
|
||||||
|
let cold_ms = t0.elapsed().as_millis();
|
||||||
|
|
||||||
|
let t1 = std::time::Instant::now();
|
||||||
|
let warm = manifest_from_stopped_container_cached(&id).await;
|
||||||
|
let warm_ms = t1.elapsed().as_millis();
|
||||||
|
|
||||||
|
// Restart, change the filesystem, stop again — `FinishedAt` moves.
|
||||||
|
docker_cli(&["start", &id]);
|
||||||
|
docker_cli(&["exec", &id, "mkdir", "-p", &second]);
|
||||||
|
docker_cli(&["stop", "-t", "1", &id]);
|
||||||
|
let after_restart = manifest_from_stopped_container_cached(&id).await;
|
||||||
|
|
||||||
|
cleanup();
|
||||||
|
|
||||||
|
let has = |m: &Manifest, p: &str| m.paths.iter().any(|e| e.path == p && e.is_dir());
|
||||||
|
|
||||||
|
let cold = cold.expect("cold read");
|
||||||
|
let warm = warm.expect("warm read");
|
||||||
|
let after_restart = after_restart.expect("read after restart");
|
||||||
|
|
||||||
|
assert!(has(&cold, &first), "cold read missed {}", first);
|
||||||
|
assert!(has(&warm, &first), "warm read missed {}", first);
|
||||||
|
println!("cold {} ms, warm {} ms", cold_ms, warm_ms);
|
||||||
|
assert!(
|
||||||
|
warm_ms * 5 < cold_ms.max(5),
|
||||||
|
"the second read cost {} ms against a cold {} ms — it re-committed \
|
||||||
|
instead of using the cache",
|
||||||
|
warm_ms,
|
||||||
|
cold_ms
|
||||||
|
);
|
||||||
|
|
||||||
|
// The restart must have invalidated it: the new directory has to show up.
|
||||||
|
assert!(
|
||||||
|
has(&after_restart, &second),
|
||||||
|
"a restart did not invalidate the cache — {} is missing, so this is \
|
||||||
|
a stale manifest of a filesystem the container no longer has",
|
||||||
|
second
|
||||||
|
);
|
||||||
|
assert!(has(&after_restart, &first), "the restart lost {}", first);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The reaper finds a leftover probe image by prefix and — crucially —
|
||||||
|
/// refuses to remove a young one, because that image may be another
|
||||||
|
/// Triple-C instance's live probe. Only a real daemon can say whether the
|
||||||
|
/// `reference=` glob matches the names `get_probe_image_name` produces.
|
||||||
|
///
|
||||||
|
/// The fixture is **committed**, not tagged and not built. An image's
|
||||||
|
/// `Created` is its own, not its tag's, so tagging something already on disk
|
||||||
|
/// into this namespace yields a fixture the reaper is right to call ancient
|
||||||
|
/// — and BuildKit stamps a fixed epoch on `docker build` output, so a built
|
||||||
|
/// one looks ancient too. A commit stamps *now*, verified against Engine
|
||||||
|
/// 29.6, which is also how real probe images get their age.
|
||||||
|
///
|
||||||
|
/// Both of those mistakes were made here first, and one of them deleted an
|
||||||
|
/// unrelated `alpine:latest` — which is why `reap_probe_images` removes by
|
||||||
|
/// tag rather than by image id.
|
||||||
|
///
|
||||||
|
/// ```text
|
||||||
|
/// cargo test -- --ignored --nocapture reaper_spares
|
||||||
|
/// ```
|
||||||
|
#[cfg(unix)]
|
||||||
|
#[tokio::test]
|
||||||
|
#[ignore = "needs a Docker daemon; builds and removes a throwaway image"]
|
||||||
|
async fn the_reaper_spares_a_probe_image_young_enough_to_be_someone_elses() {
|
||||||
|
use std::process::Command;
|
||||||
|
|
||||||
|
fn docker_out(args: &[&str]) -> std::process::Output {
|
||||||
|
Command::new("docker").args(args).output().expect("docker CLI")
|
||||||
|
}
|
||||||
|
|
||||||
|
let base = std::env::var("TRIPLE_C_TEST_IMAGE")
|
||||||
|
.unwrap_or_else(|_| "alpine:latest".to_string());
|
||||||
|
let name = crate::docker::container::get_probe_image_name("reapertest01234");
|
||||||
|
|
||||||
|
// A never-started container is enough to commit from, and leaves the
|
||||||
|
// daemon's run state alone entirely.
|
||||||
|
let created = docker_out(&["create", &base, "true"]);
|
||||||
|
assert!(
|
||||||
|
created.status.success(),
|
||||||
|
"could not create the fixture container from {}: {}",
|
||||||
|
base,
|
||||||
|
String::from_utf8_lossy(&created.stderr)
|
||||||
|
);
|
||||||
|
let cid = String::from_utf8_lossy(&created.stdout).trim().to_string();
|
||||||
|
|
||||||
|
let committed = docker_out(&["commit", "--pause=false", &cid, &name]);
|
||||||
|
let _ = docker_out(&["rm", "-f", &cid]);
|
||||||
|
assert!(
|
||||||
|
committed.status.success(),
|
||||||
|
"could not commit the fixture image: {}",
|
||||||
|
String::from_utf8_lossy(&committed.stderr)
|
||||||
|
);
|
||||||
|
|
||||||
|
reap_probe_images().await;
|
||||||
|
|
||||||
|
let still_there = Command::new("docker")
|
||||||
|
.args(["image", "inspect", &name])
|
||||||
|
.output()
|
||||||
|
.expect("docker image inspect")
|
||||||
|
.status
|
||||||
|
.success();
|
||||||
|
|
||||||
|
let _ = Command::new("docker").args(["rmi", &name]).output();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
still_there,
|
||||||
|
"a probe image committed seconds ago was reaped — that is another \
|
||||||
|
instance's live probe being broken, see PROBE_REAP_MIN_AGE_SECS"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A *stopped* container is readable, and what comes back is its writable
|
||||||
|
/// layer rather than the image it was created from. This is the whole point
|
||||||
|
/// of the function: the base image cannot answer it, and the project may
|
||||||
|
/// well have no snapshot image at all.
|
||||||
|
///
|
||||||
|
/// Also asserts the throwaway commit leaves nothing behind, which no unit
|
||||||
|
/// test can. It has to assert on the `triple-c-probe-*` tags specifically:
|
||||||
|
/// the probe image is *tagged*, so a leak never shows up as a dangling
|
||||||
|
/// image and a dangling-set assertion here would pass either way.
|
||||||
|
///
|
||||||
|
/// Ignored because it needs Docker and commits a container; run it with
|
||||||
|
///
|
||||||
|
/// ```text
|
||||||
|
/// cargo test -- --ignored --nocapture stopped_container
|
||||||
|
/// ```
|
||||||
|
#[cfg(unix)]
|
||||||
|
#[tokio::test]
|
||||||
|
#[ignore = "needs a Docker daemon; creates, commits and removes a throwaway container"]
|
||||||
|
async fn a_stopped_container_is_read_from_its_writable_layer() {
|
||||||
|
fn docker_cli(args: &[&str]) -> String {
|
||||||
|
let out = std::process::Command::new("docker")
|
||||||
|
.args(args)
|
||||||
|
.output()
|
||||||
|
.expect("docker CLI");
|
||||||
|
assert!(
|
||||||
|
out.status.success(),
|
||||||
|
"docker {:?} failed: {}",
|
||||||
|
args,
|
||||||
|
String::from_utf8_lossy(&out.stderr)
|
||||||
|
);
|
||||||
|
String::from_utf8_lossy(&out.stdout).trim().to_string()
|
||||||
|
}
|
||||||
|
fn probe_images() -> Vec<String> {
|
||||||
|
let mut ids: Vec<String> = docker_cli(&[
|
||||||
|
"images", "-q",
|
||||||
|
"--filter",
|
||||||
|
&format!("reference={}*", crate::docker::container::PROBE_IMAGE_PREFIX),
|
||||||
|
])
|
||||||
|
.lines()
|
||||||
|
.map(|l| l.trim().to_string())
|
||||||
|
.filter(|l| !l.is_empty())
|
||||||
|
.collect();
|
||||||
|
ids.sort();
|
||||||
|
ids
|
||||||
|
}
|
||||||
|
|
||||||
|
let image = std::env::var("TRIPLE_C_TEST_IMAGE")
|
||||||
|
.unwrap_or_else(|_| "ghcr.io/shadowdao/triple-c-sandbox:latest".to_string());
|
||||||
|
// A marker only the writable layer can carry, under a MANIFEST_ROOTS root.
|
||||||
|
let marker = format!("/opt/probe-marker-{}", std::process::id());
|
||||||
|
|
||||||
|
// Another instance's live probe images are allowed to exist; what must
|
||||||
|
// hold is that this probe adds none of its own.
|
||||||
|
let before = probe_images();
|
||||||
|
|
||||||
|
let id = docker_cli(&[
|
||||||
|
"run", "-d", "--label", "triple-c.managed=true",
|
||||||
|
"--entrypoint", "/bin/sh",
|
||||||
|
&image, "-c", "sleep 300",
|
||||||
|
]);
|
||||||
|
let cleanup = |id: &str| {
|
||||||
|
let _ = std::process::Command::new("docker")
|
||||||
|
.args(["rm", "-f", id])
|
||||||
|
.output();
|
||||||
|
};
|
||||||
|
|
||||||
|
docker_cli(&["exec", &id, "mkdir", "-p", &marker]);
|
||||||
|
docker_cli(&["stop", "-t", "1", &id]);
|
||||||
|
|
||||||
|
let result = manifest_from_stopped_container(&id).await;
|
||||||
|
|
||||||
|
cleanup(&id);
|
||||||
|
|
||||||
|
let manifest = result.expect("a stopped container must be probeable");
|
||||||
|
assert!(
|
||||||
|
manifest.paths.iter().any(|e| e.path == marker && e.is_dir()),
|
||||||
|
"the probe read the image, not the container's writable layer: {} missing",
|
||||||
|
marker
|
||||||
|
);
|
||||||
|
// Non-empty package sets prove the probe script really ran, rather than
|
||||||
|
// parsing an empty transcript into an empty-but-Ok manifest.
|
||||||
|
assert!(
|
||||||
|
!manifest.apt_manual.is_empty(),
|
||||||
|
"apt-mark showmanual came back empty, so the probe did not run"
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
probe_images(),
|
||||||
|
before,
|
||||||
|
"the throwaway probe image was not cleaned up"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ mod logging;
|
|||||||
mod models;
|
mod models;
|
||||||
mod project_lock;
|
mod project_lock;
|
||||||
mod storage;
|
mod storage;
|
||||||
|
pub mod url_open;
|
||||||
pub mod web_terminal;
|
pub mod web_terminal;
|
||||||
|
|
||||||
use std::sync::atomic::{AtomicBool, Ordering};
|
use std::sync::atomic::{AtomicBool, Ordering};
|
||||||
@@ -263,12 +264,20 @@ pub fn run() {
|
|||||||
// logged warning rather than a failed start.
|
// logged warning rather than a failed start.
|
||||||
//
|
//
|
||||||
// Ordering matters. Probes are removed first because a probe holds
|
// Ordering matters. Probes are removed first because a probe holds
|
||||||
// an image open and the sweep will not force; pins are untagged
|
// an image open and the sweep will not force — both the probe
|
||||||
|
// containers and the probe images, the latter being the one orphan
|
||||||
|
// the sweep can never reach on its own; pins are untagged
|
||||||
// second so the images they were holding are dangling by the time
|
// second so the images they were holding are dangling by the time
|
||||||
// the sweep lists them; the sweep runs last and collects both.
|
// the sweep lists them; the sweep runs last and collects both.
|
||||||
let projects_store_for_cleanup = projects_store_setup.clone();
|
let projects_store_for_cleanup = projects_store_setup.clone();
|
||||||
tauri::async_runtime::spawn(async move {
|
tauri::async_runtime::spawn(async move {
|
||||||
crate::docker::reap_probe_containers().await;
|
crate::docker::reap_probe_containers().await;
|
||||||
|
// Probe *images* too, and for a sharper reason: a probe
|
||||||
|
// container merely pins an image the sweep then refuses to
|
||||||
|
// touch, whereas a leftover probe image is tagged and so
|
||||||
|
// nothing else in this app can ever collect it. See
|
||||||
|
// `reap_probe_images`.
|
||||||
|
crate::docker::reap_probe_images().await;
|
||||||
let reaped = crate::docker::reap_stale_migration_pins().await;
|
let reaped = crate::docker::reap_stale_migration_pins().await;
|
||||||
if reaped > 0 {
|
if reaped > 0 {
|
||||||
log::info!("Startup housekeeping dropped {} stale rollback pin(s)", reaped);
|
log::info!("Startup housekeeping dropped {} stale rollback pin(s)", reaped);
|
||||||
@@ -544,6 +553,9 @@ pub fn run() {
|
|||||||
commands::update_commands::check_image_update,
|
commands::update_commands::check_image_update,
|
||||||
// Help
|
// Help
|
||||||
commands::help_commands::get_help_content,
|
commands::help_commands::get_help_content,
|
||||||
|
// Opening a link in the host browser (see `url_open` for why this
|
||||||
|
// is not `@tauri-apps/plugin-opener` on Linux)
|
||||||
|
url_open::open_url_external,
|
||||||
// Install helper
|
// Install helper
|
||||||
commands::install_helper_commands::detect_install_options,
|
commands::install_helper_commands::detect_install_options,
|
||||||
commands::install_helper_commands::run_docker_install,
|
commands::install_helper_commands::run_docker_install,
|
||||||
@@ -926,7 +938,6 @@ mod tests {
|
|||||||
"core:webview:allow-internal-toggle-devtools",
|
"core:webview:allow-internal-toggle-devtools",
|
||||||
"dialog:allow-open",
|
"dialog:allow-open",
|
||||||
"dialog:allow-save",
|
"dialog:allow-save",
|
||||||
"opener:allow-open-url",
|
|
||||||
];
|
];
|
||||||
expected.sort();
|
expected.sort();
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
@@ -63,6 +63,12 @@
|
|||||||
/// URL; most non-WebKitGTK browsers ignore the variable entirely), but
|
/// URL; most non-WebKitGTK browsers ignore the variable entirely), but
|
||||||
/// worth knowing before chasing the "links don't open" half of triple-c#34
|
/// worth knowing before chasing the "links don't open" half of triple-c#34
|
||||||
/// as a separate, unrelated cause.
|
/// as a separate, unrelated cause.
|
||||||
|
///
|
||||||
|
/// That leak is now plugged rather than merely documented: `url_open` hands
|
||||||
|
/// the opener a child environment with this variable (and the AppImage's own
|
||||||
|
/// `LD_LIBRARY_PATH`/`GTK_PATH`/... ) restored or removed. Setting it here
|
||||||
|
/// stays process-wide because GTK/WebKitGTK need it; what changed is that the
|
||||||
|
/// children no longer inherit it.
|
||||||
#[cfg(target_os = "linux")]
|
#[cfg(target_os = "linux")]
|
||||||
const DMABUF_VAR: &str = "WEBKIT_DISABLE_DMABUF_RENDERER";
|
const DMABUF_VAR: &str = "WEBKIT_DISABLE_DMABUF_RENDERER";
|
||||||
|
|
||||||
@@ -138,6 +144,12 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn main() {
|
fn main() {
|
||||||
|
// Before *any* `std::env::set_var` — `url_open` hands a child process the
|
||||||
|
// environment this app was started with, and the workaround below is one
|
||||||
|
// of the things that must not leak into it (see triple-c#34). Anything
|
||||||
|
// added here that mutates the environment belongs after this line.
|
||||||
|
triple_c_lib::url_open::capture_pristine_environment();
|
||||||
|
|
||||||
#[cfg(target_os = "linux")]
|
#[cfg(target_os = "linux")]
|
||||||
apply_webkit_wayland_workaround();
|
apply_webkit_wayland_workaround();
|
||||||
|
|
||||||
|
|||||||
@@ -132,6 +132,26 @@ fn default_use_shared_auth_token() -> bool {
|
|||||||
true
|
true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `auth_bridge_enabled` defaults to **on**, and the default is what makes
|
||||||
|
/// `claude login` work at all.
|
||||||
|
///
|
||||||
|
/// The login flow binds a *random* ephemeral loopback port inside the
|
||||||
|
/// container and then sends the host's browser to `127.0.0.1:<that port>`.
|
||||||
|
/// On the host nothing is listening there, so the callback lands on a closed
|
||||||
|
/// port and the CLI waits for a redirect that can never arrive. The bridge
|
||||||
|
/// mirrors the container's loopback listeners onto the same host port, which
|
||||||
|
/// is the only thing that closes that loop — so off-by-default made a hang the
|
||||||
|
/// out-of-the-box experience.
|
||||||
|
///
|
||||||
|
/// Returning `true` from a `#[serde(default)]` helper (rather than flipping the
|
||||||
|
/// constructor alone) is deliberate: existing `projects.json` records were
|
||||||
|
/// written before this field existed, or while it was off, and an absent key is
|
||||||
|
/// what the default is read for. A project that wants the old behaviour turns
|
||||||
|
/// the toggle off, which persists an explicit `false`.
|
||||||
|
fn default_auth_bridge_enabled() -> bool {
|
||||||
|
true
|
||||||
|
}
|
||||||
|
|
||||||
/// How much autonomy Claude Code is granted inside the container.
|
/// How much autonomy Claude Code is granted inside the container.
|
||||||
///
|
///
|
||||||
/// Maps onto Claude Code CLI flags — see [`PermissionMode::cli_args`], which is
|
/// Maps onto Claude Code CLI flags — see [`PermissionMode::cli_args`], which is
|
||||||
@@ -336,17 +356,30 @@ pub struct Project {
|
|||||||
pub sandbox_mode_enabled: bool,
|
pub sandbox_mode_enabled: bool,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub mission_control_enabled: bool,
|
pub mission_control_enabled: bool,
|
||||||
/// Opt in to the auth bridge: while the container runs, its loopback
|
/// The auth bridge: while the container runs, its loopback listeners are
|
||||||
/// listeners are mirrored onto the host's loopback so browser OAuth
|
/// mirrored onto the host's loopback so browser OAuth callbacks
|
||||||
/// callbacks (`claude login`, `fly login`, `aws sso login`) can reach them.
|
/// (`claude login`, `fly login`, `aws sso login`) can reach them.
|
||||||
/// Purely host-side — it deliberately has no container-recreation label,
|
/// Purely host-side — it deliberately has no container-recreation label,
|
||||||
/// because toggling it changes nothing about the container itself.
|
/// because toggling it changes nothing about the container itself.
|
||||||
#[serde(default)]
|
///
|
||||||
|
/// **On by default**, and opt-*out* rather than opt-in — see
|
||||||
|
/// [`default_auth_bridge_enabled`] for why the default is the feature.
|
||||||
|
#[serde(default = "default_auth_bridge_enabled")]
|
||||||
pub auth_bridge_enabled: bool,
|
pub auth_bridge_enabled: bool,
|
||||||
/// Opt in to the browser-view pane, which watches and takes over the
|
/// Opt in to the browser-view pane, which watches and takes over the
|
||||||
/// browser Claude drives with Playwright inside the container. Purely
|
/// browser Claude drives with Playwright inside the container. Purely
|
||||||
/// host-side like `auth_bridge_enabled`, so it likewise has no
|
/// host-side like `auth_bridge_enabled`, so it likewise has no
|
||||||
/// container-recreation label.
|
/// container-recreation label.
|
||||||
|
///
|
||||||
|
/// This is the *durable* home of the flag: `BrowserViewManager` reads it
|
||||||
|
/// rather than keeping its own copy, so the pane comes back the way it was
|
||||||
|
/// left. Off by default, and unlike the auth bridge it stays that way — a
|
||||||
|
/// view costs a container exec, a Node daemon and a host port, and a
|
||||||
|
/// container without Playwright cannot serve one at all.
|
||||||
|
///
|
||||||
|
/// Durable does **not** mean auto-started: nothing brings a viewer up on
|
||||||
|
/// app start, so a project left enabled reports `enabled` with a state of
|
||||||
|
/// `Off` until the pane (or `open_page_in_container_browser`) asks for one.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub browser_view_enabled: bool,
|
pub browser_view_enabled: bool,
|
||||||
/// Grant the container what a VPN client needs to build a tunnel:
|
/// Grant the container what a VPN client needs to build a tunnel:
|
||||||
@@ -639,7 +672,7 @@ impl Project {
|
|||||||
allow_docker_access: false,
|
allow_docker_access: false,
|
||||||
sandbox_mode_enabled: false,
|
sandbox_mode_enabled: false,
|
||||||
mission_control_enabled: false,
|
mission_control_enabled: false,
|
||||||
auth_bridge_enabled: false,
|
auth_bridge_enabled: default_auth_bridge_enabled(),
|
||||||
browser_view_enabled: false,
|
browser_view_enabled: false,
|
||||||
vpn_support_enabled: false,
|
vpn_support_enabled: false,
|
||||||
use_shared_auth_token: default_use_shared_auth_token(),
|
use_shared_auth_token: default_use_shared_auth_token(),
|
||||||
@@ -885,4 +918,69 @@ mod tests {
|
|||||||
let round_tripped: ClaudeCodeSettings = serde_json::from_str(&json).unwrap();
|
let round_tripped: ClaudeCodeSettings = serde_json::from_str(&json).unwrap();
|
||||||
assert_eq!(round_tripped, partial);
|
assert_eq!(round_tripped, partial);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── The host-side per-project toggles ─────────────────────────────────
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_project_stored_before_the_auth_bridge_existed_gets_it_turned_on() {
|
||||||
|
// The whole point of the serde default: `MAIN_SHAPE_PROJECT` is a real
|
||||||
|
// record written by a shipped binary and has no `auth_bridge_enabled`
|
||||||
|
// key at all. Without this, every existing project keeps hanging on
|
||||||
|
// `claude login` until its owner finds the toggle.
|
||||||
|
assert!(!MAIN_SHAPE_PROJECT.contains("auth_bridge_enabled"));
|
||||||
|
let project: Project = serde_json::from_str(MAIN_SHAPE_PROJECT).unwrap();
|
||||||
|
assert!(project.auth_bridge_enabled);
|
||||||
|
|
||||||
|
// The browser view is the other way round and must stay so: it costs a
|
||||||
|
// Node daemon, a container exec loop and a host port, and most
|
||||||
|
// containers have no Playwright to serve it with.
|
||||||
|
assert!(!project.browser_view_enabled);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn turning_the_auth_bridge_off_survives_the_default() {
|
||||||
|
// Opt-out has to be expressible, or the toggle does nothing across a
|
||||||
|
// restart. An explicit `false` in the file beats the default.
|
||||||
|
let json = r#"{ "auth_bridge_enabled": false }"#;
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
struct JustTheFlag {
|
||||||
|
#[serde(default = "default_auth_bridge_enabled")]
|
||||||
|
auth_bridge_enabled: bool,
|
||||||
|
}
|
||||||
|
let parsed: JustTheFlag = serde_json::from_str(json).unwrap();
|
||||||
|
assert!(!parsed.auth_bridge_enabled);
|
||||||
|
|
||||||
|
// And a saved project always writes the key, so the choice is pinned
|
||||||
|
// rather than re-defaulted on the next load.
|
||||||
|
let mut p = Project::new("demo".to_string(), Vec::new());
|
||||||
|
p.auth_bridge_enabled = false;
|
||||||
|
let round_tripped: Project =
|
||||||
|
serde_json::from_str(&serde_json::to_string(&p).unwrap()).unwrap();
|
||||||
|
assert!(!round_tripped.auth_bridge_enabled);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_new_project_starts_with_the_bridge_on_and_the_view_off() {
|
||||||
|
let p = Project::new("demo".to_string(), Vec::new());
|
||||||
|
assert!(p.auth_bridge_enabled);
|
||||||
|
assert!(!p.browser_view_enabled);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_path_migration_never_writes_the_flags_and_so_cannot_defeat_the_default() {
|
||||||
|
// `ProjectsStore::new` runs every record through this before
|
||||||
|
// deserialising. If it inserted either key — even as `false` — the
|
||||||
|
// serde default above would never be consulted for an existing project
|
||||||
|
// and this change would be a no-op on exactly the projects it is for.
|
||||||
|
let legacy = serde_json::json!({
|
||||||
|
"id": "p1",
|
||||||
|
"name": "demo",
|
||||||
|
"path": "/home/u/demo",
|
||||||
|
});
|
||||||
|
let migrated = Project::migrate_from_value(legacy);
|
||||||
|
let obj = migrated.as_object().unwrap();
|
||||||
|
assert!(obj.contains_key("paths"), "the migration should still do its own job");
|
||||||
|
assert!(!obj.contains_key("auth_bridge_enabled"));
|
||||||
|
assert!(!obj.contains_key("browser_view_enabled"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -241,6 +241,21 @@ impl ProjectsStore {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Granular setter for the browser view's opt-in, for the same reason
|
||||||
|
/// [`Self::set_auth_bridge_enabled`] has one: the pane toggles this while
|
||||||
|
/// the Config tab may be holding an older copy of the whole record.
|
||||||
|
pub fn set_browser_view_enabled(&self, project_id: &str, enabled: bool) -> Result<(), String> {
|
||||||
|
let mut projects = self.lock();
|
||||||
|
if let Some(p) = projects.iter_mut().find(|p| p.id == project_id) {
|
||||||
|
p.browser_view_enabled = enabled;
|
||||||
|
p.updated_at = chrono::Utc::now().to_rfc3339();
|
||||||
|
self.save(&projects)?;
|
||||||
|
Ok(())
|
||||||
|
} else {
|
||||||
|
Err(format!("Project {} not found", project_id))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub fn set_container_id(&self, project_id: &str, container_id: Option<String>) -> Result<(), String> {
|
pub fn set_container_id(&self, project_id: &str, container_id: Option<String>) -> Result<(), String> {
|
||||||
let mut projects = self.lock();
|
let mut projects = self.lock();
|
||||||
if let Some(p) = projects.iter_mut().find(|p| p.id == project_id) {
|
if let Some(p) = projects.iter_mut().find(|p| p.id == project_id) {
|
||||||
@@ -338,4 +353,61 @@ mod tests {
|
|||||||
|
|
||||||
fs::remove_dir_all(&dir).ok();
|
fs::remove_dir_all(&dir).ok();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A store over a temp file. `new()` insists on `dirs::data_dir()`, which
|
||||||
|
/// is the real user's; the fields are right here, so the granular setters
|
||||||
|
/// can be exercised against a directory the test owns.
|
||||||
|
fn store_over(dir: &Path, projects: Vec<Project>) -> ProjectsStore {
|
||||||
|
ProjectsStore {
|
||||||
|
projects: Mutex::new(projects),
|
||||||
|
file_path: dir.join("projects.json"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_browser_view_flag_is_written_to_disk_and_read_back() {
|
||||||
|
// The point of the whole exercise: before this the flag lived in a
|
||||||
|
// `HashSet` in `BrowserViewManager` and an app restart forgot it.
|
||||||
|
let dir = temp_dir("browser-view");
|
||||||
|
let project = Project::new("demo".to_string(), Vec::new());
|
||||||
|
let id = project.id.clone();
|
||||||
|
let store = store_over(&dir, vec![project]);
|
||||||
|
|
||||||
|
assert!(!store.get(&id).unwrap().browser_view_enabled);
|
||||||
|
store.set_browser_view_enabled(&id, true).unwrap();
|
||||||
|
assert!(store.get(&id).unwrap().browser_view_enabled);
|
||||||
|
|
||||||
|
// Durable, not merely in memory — this is what a restart reads.
|
||||||
|
let on_disk: Vec<Project> =
|
||||||
|
serde_json::from_str(&fs::read_to_string(dir.join("projects.json")).unwrap()).unwrap();
|
||||||
|
assert!(on_disk[0].browser_view_enabled);
|
||||||
|
|
||||||
|
store.set_browser_view_enabled(&id, false).unwrap();
|
||||||
|
assert!(!store.get(&id).unwrap().browser_view_enabled);
|
||||||
|
|
||||||
|
assert!(store.set_browser_view_enabled("no-such-project", true).is_err());
|
||||||
|
|
||||||
|
fs::remove_dir_all(&dir).ok();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_granular_toggle_leaves_every_other_field_alone() {
|
||||||
|
// Why these setters exist at all: the Config tab can be holding an
|
||||||
|
// older copy of the whole record while the pane flips one flag.
|
||||||
|
let dir = temp_dir("granular");
|
||||||
|
let mut project = Project::new("demo".to_string(), Vec::new());
|
||||||
|
project.claude_instructions = Some("keep me".to_string());
|
||||||
|
let id = project.id.clone();
|
||||||
|
let store = store_over(&dir, vec![project]);
|
||||||
|
|
||||||
|
store.set_browser_view_enabled(&id, true).unwrap();
|
||||||
|
store.set_auth_bridge_enabled(&id, false).unwrap();
|
||||||
|
|
||||||
|
let saved = store.get(&id).unwrap();
|
||||||
|
assert_eq!(saved.claude_instructions.as_deref(), Some("keep me"));
|
||||||
|
assert!(saved.browser_view_enabled);
|
||||||
|
assert!(!saved.auth_bridge_enabled);
|
||||||
|
|
||||||
|
fs::remove_dir_all(&dir).ok();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,791 @@
|
|||||||
|
//! Opening a URL in the *host's* browser — the half of triple-c#34 where
|
||||||
|
//! "Open" appeared to do nothing on Linux.
|
||||||
|
//!
|
||||||
|
//! # Why this module exists rather than `openUrl` from `@tauri-apps/plugin-opener`
|
||||||
|
//!
|
||||||
|
//! The plugin's Linux path shells out to `xdg-open`, and the child inherits
|
||||||
|
//! this process's environment verbatim. Inside an AppImage that environment is
|
||||||
|
//! not the user's — it is the AppImage's, and it is actively hostile to any
|
||||||
|
//! program that is not the one the bundle was built for:
|
||||||
|
//!
|
||||||
|
//! - linuxdeploy's `AppRun`/`AppRun.wrapped` prepends the bundle's own
|
||||||
|
//! directories to `LD_LIBRARY_PATH`, `PATH`, `XDG_DATA_DIRS`, `PYTHONPATH`,
|
||||||
|
//! `PERLLIB`, `QT_PLUGIN_PATH` and `GSETTINGS_SCHEMA_DIR`.
|
||||||
|
//! - `linuxdeploy-plugin-gtk`'s hook adds `GTK_PATH`, `GTK_EXE_PREFIX`,
|
||||||
|
//! `GTK_DATA_PREFIX`, `GTK_IM_MODULE_FILE`, `GIO_MODULE_DIR` and
|
||||||
|
//! `GDK_PIXBUF_MODULE_FILE`.
|
||||||
|
//! - `scripts/finalize-appimage.sh` installs one more hook of our own
|
||||||
|
//! (`triple-c-wayland-fallback.sh`) that can prepend
|
||||||
|
//! `$APPDIR/usr/lib/wayland-fallback` to `LD_LIBRARY_PATH`.
|
||||||
|
//! - `main.rs` sets `WEBKIT_DISABLE_DMABUF_RENDERER` process-wide, and the
|
||||||
|
//! comment there has flagged this leak for a while: it reaches whatever the
|
||||||
|
//! app spawns afterwards.
|
||||||
|
//!
|
||||||
|
//! A browser that is *already running* is unaffected — `xdg-open` just hands
|
||||||
|
//! the URL to the existing instance over D-Bus/IPC and the new process exits.
|
||||||
|
//! A **cold-launched** browser loads our bundled GTK/glib/pixbuf stack against
|
||||||
|
//! the host's, aborts before it ever paints, and `xdg-open` has already
|
||||||
|
//! returned 0. From the app's point of view the click did nothing. That is the
|
||||||
|
//! reported symptom, and it is why the bug only reproduces for some people.
|
||||||
|
//!
|
||||||
|
//! # What this does instead
|
||||||
|
//!
|
||||||
|
//! `open_url_external` re-validates the URL (see below) and spawns the opener
|
||||||
|
//! with a **sanitized child environment**. Sanitizing is
|
||||||
|
//! [`sanitize_child_env`], a pure function over two maps so it can be tested
|
||||||
|
//! without touching process-wide state:
|
||||||
|
//!
|
||||||
|
//! 1. If the AppImage saved the pre-launch value under a `*_ORIG` /
|
||||||
|
//! `APPIMAGE_ORIGINAL_*` name, restore that. Restoring a saved original is
|
||||||
|
//! strictly better than unsetting, because the user may genuinely have had
|
||||||
|
//! an `LD_LIBRARY_PATH` of their own.
|
||||||
|
//! 2. Otherwise, if the variable differs from the value this process started
|
||||||
|
//! with, restore the start-up value. That is what undoes *our own*
|
||||||
|
//! `std::env::set_var` — `main.rs` snapshots the environment via
|
||||||
|
//! [`capture_pristine_environment`] before any mutation runs.
|
||||||
|
//! 3. Otherwise, drop only the entries that point inside `$APPDIR`, keeping
|
||||||
|
//! the rest of the list intact. Blanket-unsetting would also discard
|
||||||
|
//! whatever the user's session had set; this removes exactly the
|
||||||
|
//! bundle's own contribution.
|
||||||
|
//!
|
||||||
|
//! Nothing is invented: a variable the pristine environment did not have and
|
||||||
|
//! that does not point into `$APPDIR` is left alone, so outside an AppImage
|
||||||
|
//! (`cargo tauri dev`, a distro build) this is very close to a no-op.
|
||||||
|
//!
|
||||||
|
//! # Portal vs. `xdg-open`
|
||||||
|
//!
|
||||||
|
//! `org.freedesktop.portal.OpenURI` would sidestep both the environment leak
|
||||||
|
//! *and* a missing `x-scheme-handler/https` association, but reaching it means
|
||||||
|
//! a D-Bus client — `zbus` and its async stack — as a new dependency for one
|
||||||
|
//! call, on the only platform where we ship a single self-contained binary.
|
||||||
|
//! It also only helps where a portal is running, which is precisely the
|
||||||
|
//! desktop-environment case in which `xdg-open` already works once the
|
||||||
|
//! environment is clean. The environment *is* the bug here, so the cheap fix
|
||||||
|
//! is the complete one. `gio open` is kept as a second candidate because it
|
||||||
|
//! goes through GIO's own handler lookup rather than `xdg-open`'s shell
|
||||||
|
//! heuristics, which covers most of what the portal would have covered.
|
||||||
|
//!
|
||||||
|
//! # Security
|
||||||
|
//!
|
||||||
|
//! The URL reaching this command originates in an **untrusted container** (see
|
||||||
|
//! `app/src/lib/urlRelay.ts`). The frontend validates with `sanitizeRelayUrl`,
|
||||||
|
//! but a compromised webview can call this command directly, so the rules are
|
||||||
|
//! mirrored here and enforced again: `http`/`https` only, a non-empty host, no
|
||||||
|
//! embedded credentials, no control characters or whitespace, and a length
|
||||||
|
//! cap. The URL is never passed through a shell — `std::process::Command` with
|
||||||
|
//! explicit arguments, so there is no word-splitting, no globbing and no
|
||||||
|
//! metacharacter to escape.
|
||||||
|
|
||||||
|
use std::collections::BTreeMap;
|
||||||
|
use std::sync::OnceLock;
|
||||||
|
|
||||||
|
use url::Url;
|
||||||
|
|
||||||
|
/// Hard cap on a URL we will hand to the OS. Mirrors `MAX_RELAY_URL_LENGTH`
|
||||||
|
/// in `app/src/lib/urlRelay.ts`.
|
||||||
|
const MAX_URL_LEN: usize = 8192;
|
||||||
|
|
||||||
|
/// The environment this process was started with, captured before anything
|
||||||
|
/// mutates it. See [`capture_pristine_environment`].
|
||||||
|
// Only the Linux spawn path reads these; the macOS/Windows path delegates to
|
||||||
|
// the opener plugin. Kept unconditional (rather than `#[cfg(linux)]`) so the
|
||||||
|
// tests and the documentation stay in one piece on every platform.
|
||||||
|
#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
|
||||||
|
static PRISTINE_ENV: OnceLock<BTreeMap<String, String>> = OnceLock::new();
|
||||||
|
|
||||||
|
/// Record the environment as it was at process start.
|
||||||
|
///
|
||||||
|
/// Must be called from `main()` **before** any `std::env::set_var` — today
|
||||||
|
/// that means before `apply_webkit_wayland_workaround()`, which is the only
|
||||||
|
/// mutation in the tree. Calling it twice is harmless; the first call wins.
|
||||||
|
///
|
||||||
|
/// This is the only reliable source of truth for "what did the user actually
|
||||||
|
/// have?" for variables *we* set. It cannot recover what `AppRun` overwrote
|
||||||
|
/// before `main()` ran — that is what the `*_ORIG` and `$APPDIR` rules in
|
||||||
|
/// [`sanitize_child_env`] are for.
|
||||||
|
pub fn capture_pristine_environment() {
|
||||||
|
let _ = PRISTINE_ENV.set(std::env::vars().collect());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Variables an AppImage launcher is known to override, and that break a
|
||||||
|
/// cold-launched child that is not this app.
|
||||||
|
///
|
||||||
|
/// `PATH` is in the list for the same reason as the rest: `AppRun` prepends
|
||||||
|
/// `$APPDIR/usr/bin`, and resolving `xdg-open` (or anything the browser's own
|
||||||
|
/// wrapper script calls) out of the bundle is its own failure mode.
|
||||||
|
// Only the Linux spawn path reads these; the macOS/Windows path delegates to
|
||||||
|
// the opener plugin. Kept unconditional (rather than `#[cfg(linux)]`) so the
|
||||||
|
// tests and the documentation stay in one piece on every platform.
|
||||||
|
#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
|
||||||
|
const SANITIZED_VARS: &[&str] = &[
|
||||||
|
"GDK_PIXBUF_MODULEDIR",
|
||||||
|
"GDK_PIXBUF_MODULE_FILE",
|
||||||
|
"GIO_MODULE_DIR",
|
||||||
|
"GSETTINGS_SCHEMA_DIR",
|
||||||
|
"GTK_DATA_PREFIX",
|
||||||
|
"GTK_EXE_PREFIX",
|
||||||
|
"GTK_IM_MODULE_FILE",
|
||||||
|
"GTK_PATH",
|
||||||
|
"LD_LIBRARY_PATH",
|
||||||
|
"PATH",
|
||||||
|
"PERLLIB",
|
||||||
|
"PYTHONPATH",
|
||||||
|
"QT_PLUGIN_PATH",
|
||||||
|
"XDG_DATA_DIRS",
|
||||||
|
// Set by `main.rs`, not by AppRun — rule 2 (the pristine snapshot) is what
|
||||||
|
// removes it, since the pristine environment almost never has it.
|
||||||
|
"WEBKIT_DISABLE_DMABUF_RENDERER",
|
||||||
|
];
|
||||||
|
|
||||||
|
/// What to do to one variable in the child: `Some(value)` sets it, `None`
|
||||||
|
/// removes it.
|
||||||
|
// Only the Linux spawn path reads these; the macOS/Windows path delegates to
|
||||||
|
// the opener plugin. Kept unconditional (rather than `#[cfg(linux)]`) so the
|
||||||
|
// tests and the documentation stay in one piece on every platform.
|
||||||
|
#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
|
||||||
|
type EnvChange = (String, Option<String>);
|
||||||
|
|
||||||
|
/// True when `entry` is `appdir` itself or a path inside it.
|
||||||
|
// Only the Linux spawn path reads these; the macOS/Windows path delegates to
|
||||||
|
// the opener plugin. Kept unconditional (rather than `#[cfg(linux)]`) so the
|
||||||
|
// tests and the documentation stay in one piece on every platform.
|
||||||
|
#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
|
||||||
|
fn is_inside(entry: &str, appdir: &str) -> bool {
|
||||||
|
let appdir = appdir.trim_end_matches('/');
|
||||||
|
if appdir.is_empty() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
entry == appdir || entry.strip_prefix(appdir).is_some_and(|r| r.starts_with('/'))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Drop the `$APPDIR` entries from a colon-separated list, keeping order and
|
||||||
|
/// keeping everything else.
|
||||||
|
///
|
||||||
|
/// Single-valued variables (`GDK_PIXBUF_MODULE_FILE`, say) are just lists of
|
||||||
|
/// one, so they need no separate case: a value inside `$APPDIR` filters down
|
||||||
|
/// to nothing and the variable is removed.
|
||||||
|
// Only the Linux spawn path reads these; the macOS/Windows path delegates to
|
||||||
|
// the opener plugin. Kept unconditional (rather than `#[cfg(linux)]`) so the
|
||||||
|
// tests and the documentation stay in one piece on every platform.
|
||||||
|
#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
|
||||||
|
fn strip_appdir_entries(value: &str, appdir: &str) -> Option<String> {
|
||||||
|
let kept: Vec<&str> = value
|
||||||
|
.split(':')
|
||||||
|
.filter(|entry| !entry.is_empty() && !is_inside(entry, appdir))
|
||||||
|
.collect();
|
||||||
|
if kept.is_empty() {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
Some(kept.join(":"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Compute the changes that turn `current` into an environment safe to hand a
|
||||||
|
/// cold-launched host program.
|
||||||
|
///
|
||||||
|
/// Pure on purpose — `current` and `pristine` are passed in rather than read
|
||||||
|
/// from the process, so the rules can be tested without a global mutex around
|
||||||
|
/// the environment. Returns changes sorted by variable name so assertions are
|
||||||
|
/// deterministic.
|
||||||
|
// Only the Linux spawn path reads these; the macOS/Windows path delegates to
|
||||||
|
// the opener plugin. Kept unconditional (rather than `#[cfg(linux)]`) so the
|
||||||
|
// tests and the documentation stay in one piece on every platform.
|
||||||
|
#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
|
||||||
|
fn sanitize_child_env(
|
||||||
|
current: &BTreeMap<String, String>,
|
||||||
|
pristine: &BTreeMap<String, String>,
|
||||||
|
appdir: Option<&str>,
|
||||||
|
) -> Vec<EnvChange> {
|
||||||
|
let mut changes: Vec<EnvChange> = Vec::new();
|
||||||
|
|
||||||
|
for var in SANITIZED_VARS {
|
||||||
|
let now = current.get(*var);
|
||||||
|
|
||||||
|
// 1. A saved original always wins. Both spellings are checked because
|
||||||
|
// which one exists depends on the launcher: linuxdeploy's AppRun
|
||||||
|
// and the various `AppRun.wrapped` generations have used each.
|
||||||
|
// An empty saved value means "it was unset", not "set it to empty".
|
||||||
|
let saved = current
|
||||||
|
.get(&format!("{var}_ORIG"))
|
||||||
|
.or_else(|| current.get(&format!("APPIMAGE_ORIGINAL_{var}")));
|
||||||
|
if let Some(saved) = saved {
|
||||||
|
let restored = if saved.is_empty() {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
Some(saved.clone())
|
||||||
|
};
|
||||||
|
if restored.as_ref() != now {
|
||||||
|
changes.push((var.to_string(), restored));
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// 2. We changed it ourselves after start-up — put back what was there.
|
||||||
|
let at_start = pristine.get(*var);
|
||||||
|
if at_start != now {
|
||||||
|
changes.push((var.to_string(), at_start.cloned()));
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// 3. Polluted before `main()` ran, with nothing saved. Remove the
|
||||||
|
// bundle's own entries and keep the user's.
|
||||||
|
let (Some(now), Some(appdir)) = (now, appdir) else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
let stripped = strip_appdir_entries(now, appdir);
|
||||||
|
if stripped.as_deref() != Some(now.as_str()) {
|
||||||
|
changes.push((var.to_string(), stripped));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
changes.sort_by(|a, b| a.0.cmp(&b.0));
|
||||||
|
changes
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether `candidate` holds a character that disqualifies it before parsing.
|
||||||
|
///
|
||||||
|
/// Mirrors `hasForbiddenChar` in `app/src/lib/urlRelay.ts`, and for the same
|
||||||
|
/// reasons: C0/C1 controls and whitespace are invisible in the UI and are
|
||||||
|
/// stripped rather than rejected by some URL parsers, and quote characters are
|
||||||
|
/// illegal in a URL per RFC 3986 while being exactly what an argument-splitting
|
||||||
|
/// opener downstream would act on. Written as a scan over code points rather
|
||||||
|
/// than a regex so the control ranges cannot be mangled by an editing tool.
|
||||||
|
fn has_forbidden_char(candidate: &str) -> bool {
|
||||||
|
candidate.chars().any(|ch| {
|
||||||
|
let code = ch as u32;
|
||||||
|
code <= 0x20
|
||||||
|
|| code == 0x7f
|
||||||
|
|| (0x80..=0x9f).contains(&code)
|
||||||
|
|| ch == '"'
|
||||||
|
|| ch == '\''
|
||||||
|
|| ch == '`'
|
||||||
|
|| ch.is_whitespace()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Validate a URL an untrusted source asked the host to open.
|
||||||
|
///
|
||||||
|
/// Returns the normalized URL, or a message safe to show the user. The message
|
||||||
|
/// never echoes the input: it is the input that is untrusted, and this error
|
||||||
|
/// is rendered in a toast.
|
||||||
|
fn validate_external_url(raw: &str) -> Result<String, String> {
|
||||||
|
// Rust's `trim` strips slightly more than JavaScript's (NEL, U+0085, for
|
||||||
|
// one), so a string the frontend would have rejected can reach the parser
|
||||||
|
// here with its edges shaved. That only ever removes outer whitespace —
|
||||||
|
// everything that survives still has to pass every check below — so the
|
||||||
|
// divergence cannot widen what gets opened.
|
||||||
|
let candidate = raw.trim();
|
||||||
|
|
||||||
|
if candidate.is_empty() {
|
||||||
|
return Err("Refused to open an empty URL.".to_string());
|
||||||
|
}
|
||||||
|
if candidate.len() > MAX_URL_LEN {
|
||||||
|
return Err(format!(
|
||||||
|
"Refused to open a URL longer than {MAX_URL_LEN} characters."
|
||||||
|
));
|
||||||
|
}
|
||||||
|
if has_forbidden_char(candidate) {
|
||||||
|
return Err(
|
||||||
|
"Refused to open a URL containing whitespace, quotes or control characters."
|
||||||
|
.to_string(),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
let parsed = Url::parse(candidate).map_err(|_| "Refused to open a malformed URL.".to_string())?;
|
||||||
|
|
||||||
|
// Scheme allowlist. Nothing else, ever — `file:`, `javascript:`, `data:`
|
||||||
|
// and every registered protocol handler stay out of reach of the
|
||||||
|
// container. The scheme is safe to interpolate: the parser restricts it to
|
||||||
|
// ASCII alphanumerics, `+`, `-` and `.`.
|
||||||
|
if parsed.scheme() != "http" && parsed.scheme() != "https" {
|
||||||
|
return Err(format!(
|
||||||
|
"Refused to open a {}: URL — only http and https are allowed.",
|
||||||
|
parsed.scheme()
|
||||||
|
));
|
||||||
|
}
|
||||||
|
if parsed.host_str().is_none_or(str::is_empty) {
|
||||||
|
return Err("Refused to open a URL with no host.".to_string());
|
||||||
|
}
|
||||||
|
// `https://claude.ai@evil.tld/x` reads as claude.ai anywhere the string is
|
||||||
|
// truncated, and navigates to evil.tld.
|
||||||
|
if !parsed.username().is_empty() || parsed.password().is_some() {
|
||||||
|
return Err("Refused to open a URL containing embedded credentials.".to_string());
|
||||||
|
}
|
||||||
|
|
||||||
|
let normalized = parsed.to_string();
|
||||||
|
if normalized.len() > MAX_URL_LEN {
|
||||||
|
return Err(format!(
|
||||||
|
"Refused to open a URL longer than {MAX_URL_LEN} characters."
|
||||||
|
));
|
||||||
|
}
|
||||||
|
// A normalized http(s) URL is ASCII by construction — the host is
|
||||||
|
// punycoded and everything after it is percent-encoded. Asserting it means
|
||||||
|
// nothing non-ASCII can reach an `execvp` argument, whatever the parser
|
||||||
|
// decides to do in a future version.
|
||||||
|
if !normalized.is_ascii() {
|
||||||
|
return Err("Refused to open a URL with non-ASCII characters.".to_string());
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(normalized)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Openers to try, in order, each as (program, leading arguments).
|
||||||
|
///
|
||||||
|
/// `xdg-open` first because it is what the desktop expects to be asked and
|
||||||
|
/// honours the user's `mimeapps.list`. `gio open` second: it is present
|
||||||
|
/// wherever glib is (which, for a GTK app's host, is everywhere) and resolves
|
||||||
|
/// the handler through GIO rather than `xdg-open`'s shell heuristics, so it
|
||||||
|
/// still works when the `x-scheme-handler/https` association `xdg-open` looks
|
||||||
|
/// for is missing or points at something broken.
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
const OPENERS: &[(&str, &[&str])] = &[("xdg-open", &[]), ("gio", &["open"])];
|
||||||
|
|
||||||
|
/// How long a candidate opener is given to fail before it is assumed to have
|
||||||
|
/// worked.
|
||||||
|
///
|
||||||
|
/// `xdg-open` usually returns immediately (it hands the URL to a running
|
||||||
|
/// browser and exits), but in its generic fallback mode it *is* the browser's
|
||||||
|
/// parent and stays alive for the session. So "still running" cannot be read
|
||||||
|
/// as failure, and "exited non-zero quickly" is the only negative signal there
|
||||||
|
/// is — though not, on its own, a trustworthy one. See
|
||||||
|
/// [`exit_code_means_nothing_was_launched`].
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
const OPENER_GRACE: std::time::Duration = std::time::Duration::from_millis(400);
|
||||||
|
|
||||||
|
/// Whether a non-zero exit says the opener certainly launched nothing, and so
|
||||||
|
/// that the next candidate can be tried without risking a second tab.
|
||||||
|
///
|
||||||
|
/// The loop used to treat every quick non-zero exit as "it did nothing" and
|
||||||
|
/// fall through. That is safe for most of `xdg-open`'s documented codes — 1
|
||||||
|
/// (syntax), 2 (file not found) and 3 (a required tool could not be found) are
|
||||||
|
/// all statements that it never got as far as launching a handler, and 3 is the
|
||||||
|
/// missing-association case `gio open` is in [`OPENERS`] for. 127 is the same
|
||||||
|
/// statement made by a shell, which is how a `$BROWSER` or `x-www-browser`
|
||||||
|
/// wrapper naming a program that does not exist comes back.
|
||||||
|
///
|
||||||
|
/// Code 4 is the one that cannot be read that way, and it is the catch-all:
|
||||||
|
/// "the action failed" also covers a handler that *was* launched and then
|
||||||
|
/// returned non-zero. A browser that takes the URL, opens the tab in an already
|
||||||
|
/// running instance and exits non-zero for its own reasons ends up here, as
|
||||||
|
/// does a wrapper script that does its job and then returns the exit status of
|
||||||
|
/// something else. Falling through on that hands the same URL to a second
|
||||||
|
/// opener: two tabs for one click, and for an OAuth link two authorize
|
||||||
|
/// requests.
|
||||||
|
///
|
||||||
|
/// So anything not recognised below — 4, an unfamiliar code, or a death by
|
||||||
|
/// signal (`code()` is `None`) — ends the loop rather than continuing it. The
|
||||||
|
/// caller is told the opener failed, which is the honest report of an
|
||||||
|
/// ambiguous outcome, and no second request is made on the user's behalf. Note
|
||||||
|
/// what this costs: an opener that genuinely failed with code 4 no longer falls
|
||||||
|
/// through to `gio`, so a user whose `xdg-open` fails that way sees an error
|
||||||
|
/// where they previously might have got a tab.
|
||||||
|
///
|
||||||
|
/// This is reasoning from `xdg-open`'s documented exit codes, not from an
|
||||||
|
/// observed double-open in this app.
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
fn exit_code_means_nothing_was_launched(code: Option<i32>) -> bool {
|
||||||
|
matches!(code, Some(1 | 2 | 3 | 127))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Spawn `url` with an opener, under a sanitized environment.
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
fn spawn_with_clean_env(url: &str) -> Result<(), String> {
|
||||||
|
let current: BTreeMap<String, String> = std::env::vars().collect();
|
||||||
|
let pristine = PRISTINE_ENV.get().cloned().unwrap_or_else(|| current.clone());
|
||||||
|
let appdir = current.get("APPDIR").cloned();
|
||||||
|
let changes = sanitize_child_env(¤t, &pristine, appdir.as_deref());
|
||||||
|
|
||||||
|
let mut failures: Vec<String> = Vec::new();
|
||||||
|
|
||||||
|
for (program, leading) in OPENERS {
|
||||||
|
let mut command = std::process::Command::new(program);
|
||||||
|
command.args(*leading).arg(url);
|
||||||
|
// The bundle's own identity is not the child's business either, and a
|
||||||
|
// browser that re-execs itself through a wrapper script can pick these
|
||||||
|
// up.
|
||||||
|
for var in ["APPDIR", "APPIMAGE", "ARGV0", "OWD"] {
|
||||||
|
command.env_remove(var);
|
||||||
|
}
|
||||||
|
for (key, value) in &changes {
|
||||||
|
match value {
|
||||||
|
Some(value) => command.env(key, value),
|
||||||
|
None => command.env_remove(key),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
// Detached: the opener must not inherit our stdio, or a browser
|
||||||
|
// writing to stderr keeps a pipe to us open for the session.
|
||||||
|
command
|
||||||
|
.stdin(std::process::Stdio::null())
|
||||||
|
.stdout(std::process::Stdio::null())
|
||||||
|
.stderr(std::process::Stdio::null());
|
||||||
|
|
||||||
|
// A spawn failure — `ErrorKind::NotFound` for an opener that is not
|
||||||
|
// installed, `PermissionDenied` for one that cannot be executed — is
|
||||||
|
// the unambiguous case: nothing ran, so nothing was opened, and the
|
||||||
|
// next candidate is free to try.
|
||||||
|
let mut child = match command.spawn() {
|
||||||
|
Ok(child) => child,
|
||||||
|
Err(err) => {
|
||||||
|
failures.push(format!("{program}: {err}"));
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
std::thread::sleep(OPENER_GRACE);
|
||||||
|
match child.try_wait() {
|
||||||
|
Ok(Some(status)) if !status.success() => {
|
||||||
|
failures.push(format!("{program} exited with {status}"));
|
||||||
|
// A program that *ran* is not a program that did nothing.
|
||||||
|
if !exit_code_means_nothing_was_launched(status.code()) {
|
||||||
|
return Err(format!(
|
||||||
|
"Could not confirm the link opened. Tried: {}. It may have opened anyway \
|
||||||
|
— check your browser before trying again.",
|
||||||
|
failures.join("; ")
|
||||||
|
));
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
Ok(_) => {}
|
||||||
|
Err(err) => {
|
||||||
|
failures.push(format!("{program}: could not be waited on: {err}"));
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Still running (it is the browser's parent) — reap it off-thread so it
|
||||||
|
// does not become a zombie for the life of the app.
|
||||||
|
std::thread::spawn(move || {
|
||||||
|
let _ = child.wait();
|
||||||
|
});
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
|
||||||
|
Err(format!(
|
||||||
|
"Could not open the link. Tried: {}. Check that xdg-utils is installed and that a default browser is set.",
|
||||||
|
failures.join("; ")
|
||||||
|
))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Open `url` in the user's browser.
|
||||||
|
///
|
||||||
|
/// On Linux this goes through [`spawn_with_clean_env`] rather than
|
||||||
|
/// `@tauri-apps/plugin-opener`, for the AppImage reasons in this module's
|
||||||
|
/// documentation (triple-c#34). macOS and Windows keep the plugin's path —
|
||||||
|
/// neither has the environment problem, and `open`/`ShellExecute` are the
|
||||||
|
/// right calls there — but they are reached through this same command so the
|
||||||
|
/// frontend has one call site with one set of validation rules.
|
||||||
|
///
|
||||||
|
/// Errors are returned rather than logged-and-swallowed: "Open" silently doing
|
||||||
|
/// nothing is the bug being fixed, so the failure has to be something the UI
|
||||||
|
/// can show.
|
||||||
|
#[tauri::command]
|
||||||
|
pub async fn open_url_external(app: tauri::AppHandle, url: String) -> Result<(), String> {
|
||||||
|
let validated = validate_external_url(&url)?;
|
||||||
|
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
{
|
||||||
|
let _ = &app;
|
||||||
|
tauri::async_runtime::spawn_blocking(move || spawn_with_clean_env(&validated))
|
||||||
|
.await
|
||||||
|
.map_err(|err| format!("Could not open the link: {err}"))?
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(not(target_os = "linux"))]
|
||||||
|
{
|
||||||
|
use tauri_plugin_opener::OpenerExt;
|
||||||
|
app.opener()
|
||||||
|
.open_url(validated, None::<&str>)
|
||||||
|
.map_err(|err| format!("Could not open the link: {err}"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
fn map(pairs: &[(&str, &str)]) -> BTreeMap<String, String> {
|
||||||
|
pairs
|
||||||
|
.iter()
|
||||||
|
.map(|(k, v)| (k.to_string(), v.to_string()))
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── URL re-validation ────────────────────────────────────────────────
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn plain_http_and_https_urls_are_accepted() {
|
||||||
|
for url in [
|
||||||
|
"https://claude.ai/",
|
||||||
|
"http://localhost:1420/callback?code=abc",
|
||||||
|
"https://example.com/path#frag",
|
||||||
|
] {
|
||||||
|
assert!(validate_external_url(url).is_ok(), "{url} should be allowed");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn urls_are_returned_normalized() {
|
||||||
|
assert_eq!(
|
||||||
|
validate_external_url("https://Example.COM").unwrap(),
|
||||||
|
"https://example.com/"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn only_http_and_https_survive() {
|
||||||
|
for url in [
|
||||||
|
"file:///etc/passwd",
|
||||||
|
"javascript:alert(1)",
|
||||||
|
"data:text/html,<script>",
|
||||||
|
"ftp://example.com/x",
|
||||||
|
"vscode://foo/bar",
|
||||||
|
"mailto:someone@example.com",
|
||||||
|
] {
|
||||||
|
assert!(
|
||||||
|
validate_external_url(url).is_err(),
|
||||||
|
"{url} must not be openable"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn embedded_credentials_are_refused() {
|
||||||
|
for url in [
|
||||||
|
"https://claude.ai@evil.tld/x",
|
||||||
|
"https://user:pass@example.com/",
|
||||||
|
"https://:pass@example.com/",
|
||||||
|
] {
|
||||||
|
assert!(
|
||||||
|
validate_external_url(url).is_err(),
|
||||||
|
"{url} must not be openable"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn control_characters_and_whitespace_are_refused() {
|
||||||
|
// `\n` in particular: parsers that strip it would turn the first of
|
||||||
|
// these into a `javascript:` URL.
|
||||||
|
for url in [
|
||||||
|
"java\nscript:alert(1)",
|
||||||
|
"https://example.com/\u{7f}",
|
||||||
|
"https://example.com/\u{85}x",
|
||||||
|
"https://example.com/a b",
|
||||||
|
"https://example.com/\u{00a0}x",
|
||||||
|
"https://example.com/\"",
|
||||||
|
"https://example.com/'",
|
||||||
|
"https://example.com/`",
|
||||||
|
] {
|
||||||
|
assert!(
|
||||||
|
validate_external_url(url).is_err(),
|
||||||
|
"{url:?} must not be openable"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn empty_and_oversized_are_refused() {
|
||||||
|
assert!(validate_external_url("").is_err());
|
||||||
|
assert!(validate_external_url(" ").is_err());
|
||||||
|
let long = format!("https://example.com/{}", "a".repeat(MAX_URL_LEN));
|
||||||
|
assert!(validate_external_url(&long).is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_host_is_required() {
|
||||||
|
assert!(validate_external_url("https://").is_err());
|
||||||
|
assert!(validate_external_url("http://:8080/").is_err());
|
||||||
|
// Not a missing host: WHATWG's "special authority ignore slashes"
|
||||||
|
// state eats the third slash, so this is the host `path` in both
|
||||||
|
// `new URL()` and here. Asserted so the parity is on the record.
|
||||||
|
assert_eq!(
|
||||||
|
validate_external_url("http:///path").unwrap(),
|
||||||
|
"http://path/"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn error_messages_never_echo_the_input() {
|
||||||
|
// The input is attacker-controlled and the message goes into a toast.
|
||||||
|
let err = validate_external_url("file:///home/someone/.ssh/id_rsa").unwrap_err();
|
||||||
|
assert!(!err.contains("id_rsa"), "message leaked the input: {err}");
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Environment sanitization ─────────────────────────────────────────
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn appdir_entries_are_stripped_and_the_users_own_are_kept() {
|
||||||
|
let current = map(&[
|
||||||
|
("APPDIR", "/tmp/.mount_abc"),
|
||||||
|
("LD_LIBRARY_PATH", "/tmp/.mount_abc/usr/lib:/opt/mine/lib"),
|
||||||
|
("XDG_DATA_DIRS", "/tmp/.mount_abc/usr/share:/usr/share"),
|
||||||
|
]);
|
||||||
|
let changes = sanitize_child_env(¤t, ¤t, Some("/tmp/.mount_abc"));
|
||||||
|
assert_eq!(
|
||||||
|
changes,
|
||||||
|
vec![
|
||||||
|
(
|
||||||
|
"LD_LIBRARY_PATH".to_string(),
|
||||||
|
Some("/opt/mine/lib".to_string())
|
||||||
|
),
|
||||||
|
("XDG_DATA_DIRS".to_string(), Some("/usr/share".to_string())),
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_variable_that_is_entirely_appdir_is_removed() {
|
||||||
|
let current = map(&[
|
||||||
|
("APPDIR", "/tmp/.mount_abc"),
|
||||||
|
("GTK_PATH", "/tmp/.mount_abc/usr/lib/gtk-3.0"),
|
||||||
|
(
|
||||||
|
"GDK_PIXBUF_MODULE_FILE",
|
||||||
|
"/tmp/.mount_abc/usr/lib/gdk-pixbuf/loaders.cache",
|
||||||
|
),
|
||||||
|
]);
|
||||||
|
let changes = sanitize_child_env(¤t, ¤t, Some("/tmp/.mount_abc"));
|
||||||
|
assert_eq!(
|
||||||
|
changes,
|
||||||
|
vec![
|
||||||
|
("GDK_PIXBUF_MODULE_FILE".to_string(), None),
|
||||||
|
("GTK_PATH".to_string(), None),
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_saved_original_is_restored_rather_than_unset() {
|
||||||
|
// Restoring beats unsetting: the user may have had one of their own.
|
||||||
|
for saved_as in ["LD_LIBRARY_PATH_ORIG", "APPIMAGE_ORIGINAL_LD_LIBRARY_PATH"] {
|
||||||
|
let current = map(&[
|
||||||
|
("APPDIR", "/tmp/.mount_abc"),
|
||||||
|
("LD_LIBRARY_PATH", "/tmp/.mount_abc/usr/lib"),
|
||||||
|
(saved_as, "/home/someone/lib"),
|
||||||
|
]);
|
||||||
|
let changes = sanitize_child_env(¤t, ¤t, Some("/tmp/.mount_abc"));
|
||||||
|
assert_eq!(
|
||||||
|
changes,
|
||||||
|
vec![(
|
||||||
|
"LD_LIBRARY_PATH".to_string(),
|
||||||
|
Some("/home/someone/lib".to_string())
|
||||||
|
)],
|
||||||
|
"{saved_as} should be restored"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_empty_saved_original_means_it_was_unset() {
|
||||||
|
let current = map(&[
|
||||||
|
("APPDIR", "/tmp/.mount_abc"),
|
||||||
|
("LD_LIBRARY_PATH", "/tmp/.mount_abc/usr/lib"),
|
||||||
|
("LD_LIBRARY_PATH_ORIG", ""),
|
||||||
|
]);
|
||||||
|
let changes = sanitize_child_env(¤t, ¤t, Some("/tmp/.mount_abc"));
|
||||||
|
assert_eq!(changes, vec![("LD_LIBRARY_PATH".to_string(), None)]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn our_own_set_var_is_undone_from_the_pristine_snapshot() {
|
||||||
|
// The leak `main.rs` documents: we set this after start-up, so the
|
||||||
|
// start-up snapshot is what says it should not exist at all.
|
||||||
|
let pristine = map(&[("HOME", "/home/someone")]);
|
||||||
|
let current = map(&[
|
||||||
|
("HOME", "/home/someone"),
|
||||||
|
("WEBKIT_DISABLE_DMABUF_RENDERER", "1"),
|
||||||
|
]);
|
||||||
|
let changes = sanitize_child_env(¤t, &pristine, None);
|
||||||
|
assert_eq!(
|
||||||
|
changes,
|
||||||
|
vec![("WEBKIT_DISABLE_DMABUF_RENDERER".to_string(), None)]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_value_the_user_set_themselves_is_left_alone() {
|
||||||
|
let pristine = map(&[("WEBKIT_DISABLE_DMABUF_RENDERER", "1")]);
|
||||||
|
let current = pristine.clone();
|
||||||
|
assert!(sanitize_child_env(¤t, &pristine, None).is_empty());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn outside_an_appimage_nothing_is_touched() {
|
||||||
|
let env = map(&[
|
||||||
|
("PATH", "/usr/bin:/bin"),
|
||||||
|
("LD_LIBRARY_PATH", "/opt/mine/lib"),
|
||||||
|
("XDG_DATA_DIRS", "/usr/share"),
|
||||||
|
]);
|
||||||
|
assert!(
|
||||||
|
sanitize_child_env(&env, &env, None).is_empty(),
|
||||||
|
"a dev build or distro build must not have its environment rewritten"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn nothing_is_invented_for_variables_that_were_never_set() {
|
||||||
|
let env = map(&[("APPDIR", "/tmp/.mount_abc")]);
|
||||||
|
assert!(sanitize_child_env(&env, &env, Some("/tmp/.mount_abc")).is_empty());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_prefix_that_merely_looks_like_appdir_is_not_stripped() {
|
||||||
|
// `/tmp/.mount_abc-other` is not inside `/tmp/.mount_abc`.
|
||||||
|
let env = map(&[
|
||||||
|
("APPDIR", "/tmp/.mount_abc"),
|
||||||
|
("LD_LIBRARY_PATH", "/tmp/.mount_abc-other/lib"),
|
||||||
|
]);
|
||||||
|
assert!(sanitize_child_env(&env, &env, Some("/tmp/.mount_abc")).is_empty());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_trailing_slash_on_appdir_still_matches() {
|
||||||
|
let env = map(&[
|
||||||
|
("APPDIR", "/tmp/.mount_abc/"),
|
||||||
|
("GTK_PATH", "/tmp/.mount_abc/usr/lib/gtk-3.0"),
|
||||||
|
]);
|
||||||
|
let changes = sanitize_child_env(&env, &env, Some("/tmp/.mount_abc/"));
|
||||||
|
assert_eq!(changes, vec![("GTK_PATH".to_string(), None)]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(all(test, target_os = "linux"))]
|
||||||
|
mod opener_fallback_tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
/// The codes `xdg-open` documents as "nothing was launched". Falling
|
||||||
|
/// through to the next opener on these is what keeps `gio open` reachable
|
||||||
|
/// for the case it was added for: no usable `x-scheme-handler/https`
|
||||||
|
/// association.
|
||||||
|
#[test]
|
||||||
|
fn the_codes_that_mean_no_handler_ran_fall_through() {
|
||||||
|
for code in [1, 2, 3, 127] {
|
||||||
|
assert!(
|
||||||
|
exit_code_means_nothing_was_launched(Some(code)),
|
||||||
|
"exit {code} means the opener never launched anything"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The regression this guards: `xdg-open` returns 4 both when it could not
|
||||||
|
/// act and when the handler it launched returned non-zero — including a
|
||||||
|
/// browser that had already opened the tab. Trying `gio open` next would
|
||||||
|
/// open it a second time, which for an OAuth URL is a second authorize
|
||||||
|
/// request.
|
||||||
|
#[test]
|
||||||
|
fn an_exit_that_may_follow_a_successful_open_does_not_fall_through() {
|
||||||
|
assert!(!exit_code_means_nothing_was_launched(Some(4)));
|
||||||
|
for code in [5, 7, 126, 255] {
|
||||||
|
assert!(
|
||||||
|
!exit_code_means_nothing_was_launched(Some(code)),
|
||||||
|
"exit {code} is not a documented 'did nothing', so it must not be assumed to be one"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Killed by a signal: `code()` is `None` and the outcome is unknowable,
|
||||||
|
/// so it is treated like any other unrecognised exit.
|
||||||
|
#[test]
|
||||||
|
fn a_death_by_signal_does_not_fall_through() {
|
||||||
|
assert!(!exit_code_means_nothing_was_launched(None));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,6 +1,6 @@
|
|||||||
import { useEffect, useState } from "react";
|
import { useEffect, useState } from "react";
|
||||||
import { openUrl } from "@tauri-apps/plugin-opener";
|
|
||||||
import { useInstallHelper } from "../hooks/useInstallHelper";
|
import { useInstallHelper } from "../hooks/useInstallHelper";
|
||||||
|
import { openUrlExternal } from "../lib/tauri-commands";
|
||||||
import { useDocker } from "../hooks/useDocker";
|
import { useDocker } from "../hooks/useDocker";
|
||||||
import Modal from "./ui/Modal";
|
import Modal from "./ui/Modal";
|
||||||
import Button from "./ui/Button";
|
import Button from "./ui/Button";
|
||||||
@@ -41,7 +41,7 @@ export default function DockerInstallDialog({ onClose }: Props) {
|
|||||||
const handleOpenDocs = async () => {
|
const handleOpenDocs = async () => {
|
||||||
if (!options) return;
|
if (!options) return;
|
||||||
try {
|
try {
|
||||||
await openUrl(options.docs_url);
|
await openUrlExternal(options.docs_url);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
console.error("Failed to open docs URL:", e);
|
console.error("Failed to open docs URL:", e);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import {
|
|||||||
setBrowserViewMatchWindow,
|
setBrowserViewMatchWindow,
|
||||||
setBrowserViewPopoutAlwaysOnTop,
|
setBrowserViewPopoutAlwaysOnTop,
|
||||||
} from "../../../lib/tauri-commands";
|
} from "../../../lib/tauri-commands";
|
||||||
|
import { isBrowserViewUsable } from "../../../lib/browserViewSupport";
|
||||||
import { useAppState } from "../../../store/appState";
|
import { useAppState } from "../../../store/appState";
|
||||||
import OpenPageDialog from "./OpenPageDialog";
|
import OpenPageDialog from "./OpenPageDialog";
|
||||||
import AccordionSection from "../../ui/AccordionSection";
|
import AccordionSection from "../../ui/AccordionSection";
|
||||||
@@ -338,7 +339,7 @@ export default function BrowserTab({ project, active }: Props) {
|
|||||||
// Prefer the probe: it is the fresher of the two, and it is the one that
|
// Prefer the probe: it is the fresher of the two, and it is the one that
|
||||||
// reflects an install that just finished.
|
// reflects an install that just finished.
|
||||||
const probed = detection ?? status.detection;
|
const probed = detection ?? status.detection;
|
||||||
const ready = isUsable(probed);
|
const ready = isBrowserViewUsable(probed);
|
||||||
// Mirrors Rust `PlaywrightDetection::needs_browser`: the Chrome channel is an
|
// Mirrors Rust `PlaywrightDetection::needs_browser`: the Chrome channel is an
|
||||||
// apt package, so it never shows up in `browsers`, and a container that has
|
// apt package, so it never shows up in `browsers`, and a container that has
|
||||||
// it is not missing a browser.
|
// it is not missing a browser.
|
||||||
@@ -539,11 +540,6 @@ export default function BrowserTab({ project, active }: Props) {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Mirrors Rust `PlaywrightDetection::is_usable`. */
|
|
||||||
function isUsable(d: PlaywrightDetection | null): boolean {
|
|
||||||
return d !== null && d.playwright_version !== null && d.has_bind && d.cli_entry !== null;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Mirrors Rust `PlaywrightDetection::revision_skew`.
|
* Mirrors Rust `PlaywrightDetection::revision_skew`.
|
||||||
*
|
*
|
||||||
@@ -627,7 +623,7 @@ function Setup({
|
|||||||
onInstall: (which: Exclude<SetupJob, null>) => void;
|
onInstall: (which: Exclude<SetupJob, null>) => void;
|
||||||
}) {
|
}) {
|
||||||
const busy = job !== null;
|
const busy = job !== null;
|
||||||
const havePackages = isUsable(detection);
|
const havePackages = isBrowserViewUsable(detection);
|
||||||
const missing = missingParts(detection);
|
const missing = missingParts(detection);
|
||||||
const browsers = detection?.browsers ?? [];
|
const browsers = detection?.browsers ?? [];
|
||||||
const chrome = detection?.chrome_channel ?? null;
|
const chrome = detection?.chrome_channel ?? null;
|
||||||
|
|||||||
@@ -11,14 +11,12 @@ vi.mock("../../lib/tauri-commands", () => ({
|
|||||||
hasClaudeToken: vi.fn(),
|
hasClaudeToken: vi.fn(),
|
||||||
clearClaudeToken: vi.fn(),
|
clearClaudeToken: vi.fn(),
|
||||||
cancelClaudeToken: (...args: unknown[]) => cancelClaudeToken(...args),
|
cancelClaudeToken: (...args: unknown[]) => cancelClaudeToken(...args),
|
||||||
|
openUrlExternal: (...args: unknown[]) => openUrlExternal(...args),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
const cancelClaudeToken = vi.fn(() => Promise.resolve());
|
const cancelClaudeToken = vi.fn(() => Promise.resolve());
|
||||||
|
|
||||||
const openUrl = vi.fn();
|
const openUrlExternal = vi.fn();
|
||||||
vi.mock("@tauri-apps/plugin-opener", () => ({
|
|
||||||
openUrl: (...args: unknown[]) => openUrl(...args),
|
|
||||||
}));
|
|
||||||
|
|
||||||
/** Captured event handlers, keyed by event name, so tests can emit. */
|
/** Captured event handlers, keyed by event name, so tests can emit. */
|
||||||
const handlers = new Map<string, (event: { payload: unknown }) => void>();
|
const handlers = new Map<string, (event: { payload: unknown }) => void>();
|
||||||
@@ -174,7 +172,7 @@ describe("ClaudeAuthModal", () => {
|
|||||||
|
|
||||||
const link = await screen.findByRole("link", { name: url });
|
const link = await screen.findByRole("link", { name: url });
|
||||||
fireEvent.click(link);
|
fireEvent.click(link);
|
||||||
await waitFor(() => expect(openUrl).toHaveBeenCalledWith(url));
|
await waitFor(() => expect(openUrlExternal).toHaveBeenCalledWith(url));
|
||||||
});
|
});
|
||||||
|
|
||||||
it("ignores output belonging to a different project", async () => {
|
it("ignores output belonging to a different project", async () => {
|
||||||
@@ -259,8 +257,8 @@ describe("ClaudeAuthModal", () => {
|
|||||||
|
|
||||||
const link = await screen.findByRole("link", { name: FULL_URL });
|
const link = await screen.findByRole("link", { name: FULL_URL });
|
||||||
fireEvent.click(link);
|
fireEvent.click(link);
|
||||||
await waitFor(() => expect(openUrl).toHaveBeenCalledWith(FULL_URL));
|
await waitFor(() => expect(openUrlExternal).toHaveBeenCalledWith(FULL_URL));
|
||||||
expect(openUrl).not.toHaveBeenCalledWith(TRUNCATED_URL);
|
expect(openUrlExternal).not.toHaveBeenCalledWith(TRUNCATED_URL);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("refuses a hyperlink target that is not an Anthropic sign-in address", async () => {
|
it("refuses a hyperlink target that is not an Anthropic sign-in address", async () => {
|
||||||
@@ -270,7 +268,7 @@ describe("ClaudeAuthModal", () => {
|
|||||||
emitLink("https://evil.tld/cai/oauth/authorize?code=true");
|
emitLink("https://evil.tld/cai/oauth/authorize?code=true");
|
||||||
|
|
||||||
expect(screen.queryByRole("link")).not.toBeInTheDocument();
|
expect(screen.queryByRole("link")).not.toBeInTheDocument();
|
||||||
expect(openUrl).not.toHaveBeenCalled();
|
expect(openUrlExternal).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("ignores a hyperlink belonging to a different project", async () => {
|
it("ignores a hyperlink belonging to a different project", async () => {
|
||||||
|
|||||||
@@ -1,6 +1,5 @@
|
|||||||
import { useCallback, useEffect, useRef, useState } from "react";
|
import { useCallback, useEffect, useRef, useState } from "react";
|
||||||
import { openUrl } from "@tauri-apps/plugin-opener";
|
import { cancelClaudeToken, openUrlExternal } from "../../lib/tauri-commands";
|
||||||
import { cancelClaudeToken } from "../../lib/tauri-commands";
|
|
||||||
import Modal from "../ui/Modal";
|
import Modal from "../ui/Modal";
|
||||||
import Button from "../ui/Button";
|
import Button from "../ui/Button";
|
||||||
import StatusIndicator, { type StatusTone } from "../ui/StatusIndicator";
|
import StatusIndicator, { type StatusTone } from "../ui/StatusIndicator";
|
||||||
@@ -118,7 +117,7 @@ export default function ClaudeAuthModal({
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
await openUrl(target);
|
await openUrlExternal(target);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
setLinkError(
|
setLinkError(
|
||||||
authErrorMessage(
|
authErrorMessage(
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { openUrl } from "@tauri-apps/plugin-opener";
|
|
||||||
import type { UpdateInfo } from "../../lib/types";
|
import type { UpdateInfo } from "../../lib/types";
|
||||||
|
import { openUrlExternal } from "../../lib/tauri-commands";
|
||||||
import Modal from "../ui/Modal";
|
import Modal from "../ui/Modal";
|
||||||
import Button from "../ui/Button";
|
import Button from "../ui/Button";
|
||||||
import { formatBytes } from "../../lib/formatBytes";
|
import { formatBytes } from "../../lib/formatBytes";
|
||||||
@@ -19,7 +19,7 @@ export default function UpdateDialog({
|
|||||||
}: Props) {
|
}: Props) {
|
||||||
const handleDownload = async (url: string) => {
|
const handleDownload = async (url: string) => {
|
||||||
try {
|
try {
|
||||||
await openUrl(url);
|
await openUrlExternal(url);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
console.error("Failed to open URL:", e);
|
console.error("Failed to open URL:", e);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,7 +2,15 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
|||||||
import { render, fireEvent, cleanup, act } from "@testing-library/react";
|
import { render, fireEvent, cleanup, act } from "@testing-library/react";
|
||||||
import TerminalView, { supersedes } from "./TerminalView";
|
import TerminalView, { supersedes } from "./TerminalView";
|
||||||
import { useAppState } from "../../store/appState";
|
import { useAppState } from "../../store/appState";
|
||||||
import { uploadHostFileToTerminal } from "../../lib/tauri-commands";
|
import {
|
||||||
|
uploadHostFileToTerminal,
|
||||||
|
openUrlExternal,
|
||||||
|
} from "../../lib/tauri-commands";
|
||||||
|
import {
|
||||||
|
chooseSignInTarget,
|
||||||
|
resetBrowserSupportCache,
|
||||||
|
} from "../../hooks/useSignInOpenTarget";
|
||||||
|
import type { AuthBridgeStatus, PlaywrightDetection } from "../../lib/types";
|
||||||
import { URL_TOAST_SELECTOR } from "./UrlToast";
|
import { URL_TOAST_SELECTOR } from "./UrlToast";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -16,6 +24,18 @@ const dragDrop = vi.hoisted(() => ({
|
|||||||
handler: null as null | ((event: unknown) => unknown),
|
handler: null as null | ((event: unknown) => unknown),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
/**
|
||||||
|
* What the project's container answers about itself.
|
||||||
|
*
|
||||||
|
* `TerminalView` asks two questions on mount — is the auth bridge live, and is
|
||||||
|
* there a browser inside to open a page in — because together they decide which
|
||||||
|
* of the URL toast's two buttons leads for a sign-in link.
|
||||||
|
*/
|
||||||
|
const containerEnv = vi.hoisted(() => ({
|
||||||
|
bridge: { enabled: false, active_ports: [], conflicts: [] } as unknown,
|
||||||
|
detection: null as unknown,
|
||||||
|
}));
|
||||||
|
|
||||||
/** The `terminal-output-{id}` listeners, so a test can be the PTY. */
|
/** The `terminal-output-{id}` listeners, so a test can be the PTY. */
|
||||||
const ptyOutput = vi.hoisted(() => ({
|
const ptyOutput = vi.hoisted(() => ({
|
||||||
listeners: new Map<string, (e: { payload: number[] }) => void>(),
|
listeners: new Map<string, (e: { payload: number[] }) => void>(),
|
||||||
@@ -45,6 +65,9 @@ vi.mock("../../lib/tauri-commands", () => ({
|
|||||||
awsSsoRefresh: vi.fn(async () => {}),
|
awsSsoRefresh: vi.fn(async () => {}),
|
||||||
openPageInContainerBrowser: vi.fn(async () => ({ error: null })),
|
openPageInContainerBrowser: vi.fn(async () => ({ error: null })),
|
||||||
uploadHostFileToTerminal: vi.fn(async () => ""),
|
uploadHostFileToTerminal: vi.fn(async () => ""),
|
||||||
|
getAuthBridgeStatus: vi.fn(async () => containerEnv.bridge),
|
||||||
|
checkBrowserViewSupport: vi.fn(async () => containerEnv.detection),
|
||||||
|
openUrlExternal: vi.fn(async () => {}),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock("@tauri-apps/api/event", () => ({
|
vi.mock("@tauri-apps/api/event", () => ({
|
||||||
@@ -54,10 +77,6 @@ vi.mock("@tauri-apps/api/event", () => ({
|
|||||||
},
|
},
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock("@tauri-apps/plugin-opener", () => ({
|
|
||||||
openUrl: vi.fn(async () => {}),
|
|
||||||
}));
|
|
||||||
|
|
||||||
vi.mock("@tauri-apps/api/webview", () => ({
|
vi.mock("@tauri-apps/api/webview", () => ({
|
||||||
getCurrentWebview: () => ({
|
getCurrentWebview: () => ({
|
||||||
onDragDropEvent: async (cb: (event: unknown) => unknown) => {
|
onDragDropEvent: async (cb: (event: unknown) => unknown) => {
|
||||||
@@ -128,6 +147,14 @@ beforeEach(() => {
|
|||||||
vi.mocked(uploadHostFileToTerminal).mockResolvedValue("/workspace/api/dropped.txt");
|
vi.mocked(uploadHostFileToTerminal).mockResolvedValue("/workspace/api/dropped.txt");
|
||||||
dragDrop.handler = null;
|
dragDrop.handler = null;
|
||||||
ptyOutput.listeners.clear();
|
ptyOutput.listeners.clear();
|
||||||
|
vi.mocked(openUrlExternal).mockReset();
|
||||||
|
vi.mocked(openUrlExternal).mockResolvedValue(undefined);
|
||||||
|
containerEnv.bridge = { enabled: false, active_ports: [], conflicts: [] };
|
||||||
|
containerEnv.detection = null;
|
||||||
|
// The Playwright probe is memoized across mounts (it is a container exec), so
|
||||||
|
// a case that changes the answer has to drop what an earlier one cached.
|
||||||
|
resetBrowserSupportCache();
|
||||||
|
useAppState.setState({ toasts: [] });
|
||||||
document.body.innerHTML = "";
|
document.body.innerHTML = "";
|
||||||
useAppState.setState({ sessions: [] });
|
useAppState.setState({ sessions: [] });
|
||||||
});
|
});
|
||||||
@@ -559,6 +586,354 @@ describe("TerminalView — reaching the URL prompt without a mouse", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A container with Playwright *and* a browser in the cache — i.e. one where
|
||||||
|
* "In container" would actually open something.
|
||||||
|
*/
|
||||||
|
function usableDetection(
|
||||||
|
over: Partial<PlaywrightDetection> = {},
|
||||||
|
): PlaywrightDetection {
|
||||||
|
return {
|
||||||
|
node_version: "v22.11.0",
|
||||||
|
playwright_version: "1.56.0",
|
||||||
|
playwright_path: "/workspace/node_modules/playwright",
|
||||||
|
playwright_cli: "/workspace/node_modules/playwright/cli.js",
|
||||||
|
has_bind: true,
|
||||||
|
cli_version: "1.56.0",
|
||||||
|
cli_entry: "/workspace/node_modules/@playwright/cli/index.js",
|
||||||
|
browsers: ["chromium-1200"],
|
||||||
|
chrome_channel: null,
|
||||||
|
chromium_executable: "/home/claude/.cache/ms-playwright/chromium-1200/chrome",
|
||||||
|
chromium_executable_exists: true,
|
||||||
|
script_playwright_version: "1.56.0",
|
||||||
|
script_chromium_executable: null,
|
||||||
|
script_chromium_executable_exists: false,
|
||||||
|
searched: [],
|
||||||
|
...over,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
const LIVE_BRIDGE: AuthBridgeStatus = {
|
||||||
|
enabled: true,
|
||||||
|
active_ports: [],
|
||||||
|
conflicts: [],
|
||||||
|
};
|
||||||
|
|
||||||
|
describe("chooseSignInTarget — which action leads for a sign-in link", () => {
|
||||||
|
// The rule this replaced was "container, always", justified by the callback
|
||||||
|
// listener living inside the container. Both halves of that justification
|
||||||
|
// stopped being true: the auth bridge mirrors that listener onto the host,
|
||||||
|
// and the container-side target is Playwright's pane, whose browsers are not
|
||||||
|
// in the image.
|
||||||
|
it("prefers the host browser whenever the bridge is live", () => {
|
||||||
|
expect(chooseSignInTarget(LIVE_BRIDGE, usableDetection())).toBe("host-bridged");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not call a bridge live while it is holding a port conflict", () => {
|
||||||
|
// Enabled and unable to catch the callback anyway — the one state where
|
||||||
|
// "on" must not read as "will work".
|
||||||
|
const conflicted: AuthBridgeStatus = {
|
||||||
|
enabled: true,
|
||||||
|
active_ports: [],
|
||||||
|
conflicts: [{ port: 54545, reason: "already in use on the host" }],
|
||||||
|
};
|
||||||
|
expect(chooseSignInTarget(conflicted, usableDetection())).toBe("container");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not wait for a bridged port before trusting an enabled bridge", () => {
|
||||||
|
// There is nothing to bridge until the CLI binds its listener, and that
|
||||||
|
// races the URL reaching the transcript. Requiring a port would make the
|
||||||
|
// default flip between two identical sign-ins.
|
||||||
|
expect(chooseSignInTarget(LIVE_BRIDGE, null)).toBe("host-bridged");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls to the container only when it has a browser to open", () => {
|
||||||
|
const off: AuthBridgeStatus = { enabled: false, active_ports: [], conflicts: [] };
|
||||||
|
expect(chooseSignInTarget(off, usableDetection())).toBe("container");
|
||||||
|
// Not plain "host": with the bridge off and no browser inside, nothing is
|
||||||
|
// carrying the callback, and the toast's hint has to say so rather than
|
||||||
|
// promising a bridge. That distinction is the whole reason this answer is
|
||||||
|
// three-valued.
|
||||||
|
expect(chooseSignInTarget(off, null)).toBe("host-fallback");
|
||||||
|
// Packages installed, cache empty — the fresh-project state, and the one
|
||||||
|
// that used to be the silent default.
|
||||||
|
expect(
|
||||||
|
chooseSignInTarget(
|
||||||
|
off,
|
||||||
|
usableDetection({ browsers: [], chromium_executable_exists: false }),
|
||||||
|
),
|
||||||
|
).toBe("host-fallback");
|
||||||
|
// Playwright too old to bind: the pane cannot show it either.
|
||||||
|
expect(chooseSignInTarget(off, usableDetection({ has_bind: false }))).toBe(
|
||||||
|
"host-fallback",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("answers the host *fallback* when nothing is known at all", () => {
|
||||||
|
// "Unknown" must not read as "bridged". A status call that never answered
|
||||||
|
// is not evidence that something will carry the callback home.
|
||||||
|
expect(chooseSignInTarget(null, null)).toBe("host-fallback");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("separates a live bridge from the least-bad answer, though both lead with the host", () => {
|
||||||
|
const off: AuthBridgeStatus = { enabled: false, active_ports: [], conflicts: [] };
|
||||||
|
// The two states the old two-valued answer collapsed together. Folding them
|
||||||
|
// back into one is what let the toast tell a user with the bridge disabled
|
||||||
|
// that the bridge would carry their callback.
|
||||||
|
expect(chooseSignInTarget(LIVE_BRIDGE, null)).not.toBe(
|
||||||
|
chooseSignInTarget(off, null),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("TerminalView — the sign-in default follows the project", () => {
|
||||||
|
const SIGN_IN =
|
||||||
|
"https://claude.ai/oauth/authorize?code=true&client_id=abc&response_type=code";
|
||||||
|
|
||||||
|
function relaySequence(url: string): number[] {
|
||||||
|
return Array.from(
|
||||||
|
new TextEncoder().encode(`\x1b]7777;open;${btoa(url)}\x07`),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function mountWithPrompt() {
|
||||||
|
const view = mountSession("claude");
|
||||||
|
await act(async () => {});
|
||||||
|
const emit = ptyOutput.listeners.get("terminal-output-s1");
|
||||||
|
if (!emit) throw new Error("no terminal-output listener registered");
|
||||||
|
await act(async () => {
|
||||||
|
emit({ payload: relaySequence(SIGN_IN) });
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
});
|
||||||
|
return view;
|
||||||
|
}
|
||||||
|
|
||||||
|
function primaryLabel(): string | null {
|
||||||
|
return document.querySelector<HTMLElement>(
|
||||||
|
'[data-url-toast-primary="true"]',
|
||||||
|
)?.textContent ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function actionOrder(): (string | null)[] {
|
||||||
|
return Array.from(document.querySelectorAll("button"))
|
||||||
|
.map((b) => b.textContent)
|
||||||
|
.filter((t) => t === "Open" || t === "In container");
|
||||||
|
}
|
||||||
|
|
||||||
|
it("leads with the host browser when the auth bridge is on", async () => {
|
||||||
|
containerEnv.bridge = LIVE_BRIDGE;
|
||||||
|
containerEnv.detection = usableDetection();
|
||||||
|
await mountWithPrompt();
|
||||||
|
expect(primaryLabel()).toBe("Open");
|
||||||
|
// Both are still offered — this changes which leads, never which exist.
|
||||||
|
expect(actionOrder()).toEqual(["Open", "In container"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leads with the container when the bridge is off and a browser is there", async () => {
|
||||||
|
containerEnv.detection = usableDetection();
|
||||||
|
await mountWithPrompt();
|
||||||
|
expect(primaryLabel()).toBe("In container");
|
||||||
|
expect(actionOrder()).toEqual(["In container", "Open"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leads with the host on a fresh project, where neither is set up", async () => {
|
||||||
|
// Playwright is deliberately not baked into the image, so this is what a
|
||||||
|
// project looks like until someone presses install — and pointing the
|
||||||
|
// default at it failed on every platform, silently.
|
||||||
|
await mountWithPrompt();
|
||||||
|
expect(primaryLabel()).toBe("Open");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not promise the auth bridge on a project that has it switched off", async () => {
|
||||||
|
// The end-to-end version of the three-state answer: bridge off, no browser
|
||||||
|
// inside. The host still leads, because it is the least bad of two answers
|
||||||
|
// that can both fail — but the hint must not tell the user the bridge is
|
||||||
|
// bringing their callback home, because there is no bridge. That hint is
|
||||||
|
// what sent people to a host browser and a login that hung to its timeout.
|
||||||
|
await mountWithPrompt();
|
||||||
|
const hint = document.querySelector('[data-testid="url-toast-signin-hint"]');
|
||||||
|
expect(hint?.textContent).toMatch(/nothing is set up/i);
|
||||||
|
expect(hint?.textContent).not.toMatch(/what carries the callback/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does promise it when the bridge is actually live", async () => {
|
||||||
|
containerEnv.bridge = LIVE_BRIDGE;
|
||||||
|
await mountWithPrompt();
|
||||||
|
const hint = document.querySelector('[data-testid="url-toast-signin-hint"]');
|
||||||
|
expect(hint?.textContent).toMatch(/auth bridge/i);
|
||||||
|
expect(hint?.textContent).not.toMatch(/nothing is set up/i);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("TerminalView — a host open that fails says so", () => {
|
||||||
|
const URL = "https://github.com/login/device?code=ABCD-EFGH";
|
||||||
|
|
||||||
|
function relaySequence(url: string): number[] {
|
||||||
|
return Array.from(
|
||||||
|
new TextEncoder().encode(`\x1b]7777;open;${btoa(url)}\x07`),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function mountWithPrompt() {
|
||||||
|
const view = mountSession("claude");
|
||||||
|
await act(async () => {});
|
||||||
|
const emit = ptyOutput.listeners.get("terminal-output-s1");
|
||||||
|
if (!emit) throw new Error("no terminal-output listener registered");
|
||||||
|
await act(async () => {
|
||||||
|
emit({ payload: relaySequence(URL) });
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
});
|
||||||
|
return view;
|
||||||
|
}
|
||||||
|
|
||||||
|
function openButton(): HTMLElement {
|
||||||
|
const el = Array.from(document.querySelectorAll("button")).find(
|
||||||
|
(b) => b.textContent === "Open",
|
||||||
|
);
|
||||||
|
if (!el) throw new Error("Open button not found");
|
||||||
|
return el as HTMLElement;
|
||||||
|
}
|
||||||
|
|
||||||
|
it("pushes a toast instead of a console line nobody reads", async () => {
|
||||||
|
vi.mocked(openUrlExternal).mockRejectedValueOnce(new Error("no opener"));
|
||||||
|
await mountWithPrompt();
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.click(openButton());
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
const toasts = useAppState.getState().toasts;
|
||||||
|
expect(toasts).toHaveLength(1);
|
||||||
|
expect(toasts[0].kind).toBe("error");
|
||||||
|
expect(toasts[0].detail).toContain("no opener");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps the prompt on screen, so the other route is still one click away", async () => {
|
||||||
|
// Dismissing first is what this replaced: the toast vanished, nothing
|
||||||
|
// opened, and the URL only existed in the container's transcript.
|
||||||
|
vi.mocked(openUrlExternal).mockRejectedValueOnce(new Error("no opener"));
|
||||||
|
await mountWithPrompt();
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.click(openButton());
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
expect(document.querySelector(URL_TOAST_SELECTOR)).not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("dismisses the prompt once the handoff actually succeeded", async () => {
|
||||||
|
await mountWithPrompt();
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.click(openButton());
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
expect(openUrlExternal).toHaveBeenCalledWith(URL);
|
||||||
|
expect(document.querySelector(URL_TOAST_SELECTOR)).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("TerminalView — an open in flight must not blank a newer prompt", () => {
|
||||||
|
// The window is real and is measured in hundreds of milliseconds, not in
|
||||||
|
// microtasks: on Linux the opener sleeps `OPENER_GRACE` (400 ms, doubled when
|
||||||
|
// `xdg-open` fails and `gio` is tried) before resolving. The container is free
|
||||||
|
// to relay a second URL inside it — a `gh auth login` right after a
|
||||||
|
// `claude login` is the ordinary way that happens — and the toast slot is
|
||||||
|
// shared, so by the time the first open answers the slot may be holding a
|
||||||
|
// prompt the user has never seen. Blanking it loses that URL for good: it
|
||||||
|
// exists nowhere but the container's transcript.
|
||||||
|
const URL_A = "https://github.com/login/device?code=AAAA-1111";
|
||||||
|
const URL_B = "https://claude.ai/oauth/authorize?code=true&client_id=b";
|
||||||
|
|
||||||
|
function relaySequence(url: string): number[] {
|
||||||
|
return Array.from(
|
||||||
|
new TextEncoder().encode(`\x1b]7777;open;${btoa(url)}\x07`),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function emitRelay(url: string) {
|
||||||
|
const emit = ptyOutput.listeners.get("terminal-output-s1");
|
||||||
|
if (!emit) throw new Error("no terminal-output listener registered");
|
||||||
|
await act(async () => {
|
||||||
|
emit({ payload: relaySequence(url) });
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function openButton(): HTMLElement {
|
||||||
|
const el = Array.from(document.querySelectorAll("button")).find(
|
||||||
|
(b) => b.textContent === "Open",
|
||||||
|
);
|
||||||
|
if (!el) throw new Error("Open button not found");
|
||||||
|
return el as HTMLElement;
|
||||||
|
}
|
||||||
|
|
||||||
|
function promptedUrl(): string | null {
|
||||||
|
return (
|
||||||
|
document
|
||||||
|
.querySelector('[data-testid="url-toast-url"]')
|
||||||
|
?.getAttribute("title") ?? null
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** An `openUrlExternal` that hangs until the test lets it finish. */
|
||||||
|
function deferredOpen(): () => void {
|
||||||
|
let finish: () => void = () => {};
|
||||||
|
vi.mocked(openUrlExternal).mockReturnValueOnce(
|
||||||
|
new Promise<void>((resolve) => {
|
||||||
|
finish = () => resolve();
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
return () => finish();
|
||||||
|
}
|
||||||
|
|
||||||
|
it("keeps URL B's prompt when A's open resolves after B arrived", async () => {
|
||||||
|
const finishOpen = deferredOpen();
|
||||||
|
mountSession("claude");
|
||||||
|
await act(async () => {});
|
||||||
|
await emitRelay(URL_A);
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.click(openButton());
|
||||||
|
});
|
||||||
|
expect(openUrlExternal).toHaveBeenCalledWith(URL_A);
|
||||||
|
|
||||||
|
// The container supersedes it while the opener is still inside its grace.
|
||||||
|
await emitRelay(URL_B);
|
||||||
|
expect(promptedUrl()).toBe(URL_B);
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
finishOpen();
|
||||||
|
await Promise.resolve();
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(document.querySelector(URL_TOAST_SELECTOR)).not.toBeNull();
|
||||||
|
expect(promptedUrl()).toBe(URL_B);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still dismisses when the slot is holding the prompt that was opened", async () => {
|
||||||
|
// The other half of the guard: it must not turn "dismiss on success" into
|
||||||
|
// "never dismiss". Same deferred open, nothing superseding it.
|
||||||
|
const finishOpen = deferredOpen();
|
||||||
|
mountSession("claude");
|
||||||
|
await act(async () => {});
|
||||||
|
await emitRelay(URL_A);
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.click(openButton());
|
||||||
|
});
|
||||||
|
expect(document.querySelector(URL_TOAST_SELECTOR)).not.toBeNull();
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
finishOpen();
|
||||||
|
await Promise.resolve();
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
expect(document.querySelector(URL_TOAST_SELECTOR)).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("TerminalView — focus on request", () => {
|
describe("TerminalView — focus on request", () => {
|
||||||
/** Mount, then deliberately give focus away, so what the assertions below
|
/** Mount, then deliberately give focus away, so what the assertions below
|
||||||
* observe is the *request* taking effect and never the focus `active`
|
* observe is the *request* taking effect and never the focus `active`
|
||||||
|
|||||||
@@ -3,7 +3,6 @@ import { Terminal } from "@xterm/xterm";
|
|||||||
import { FitAddon } from "@xterm/addon-fit";
|
import { FitAddon } from "@xterm/addon-fit";
|
||||||
import { WebglAddon } from "@xterm/addon-webgl";
|
import { WebglAddon } from "@xterm/addon-webgl";
|
||||||
import { WebLinksAddon } from "@xterm/addon-web-links";
|
import { WebLinksAddon } from "@xterm/addon-web-links";
|
||||||
import { openUrl } from "@tauri-apps/plugin-opener";
|
|
||||||
import "@xterm/xterm/css/xterm.css";
|
import "@xterm/xterm/css/xterm.css";
|
||||||
import { useTerminal } from "../../hooks/useTerminal";
|
import { useTerminal } from "../../hooks/useTerminal";
|
||||||
import { useAppState } from "../../store/appState";
|
import { useAppState } from "../../store/appState";
|
||||||
@@ -11,6 +10,7 @@ import { CLAUDE_SOFT_NEWLINE } from "../../lib/claudeInput";
|
|||||||
import {
|
import {
|
||||||
awsSsoRefresh,
|
awsSsoRefresh,
|
||||||
openPageInContainerBrowser,
|
openPageInContainerBrowser,
|
||||||
|
openUrlExternal,
|
||||||
uploadHostFileToTerminal,
|
uploadHostFileToTerminal,
|
||||||
} from "../../lib/tauri-commands";
|
} from "../../lib/tauri-commands";
|
||||||
import { getCurrentWebview } from "@tauri-apps/api/webview";
|
import { getCurrentWebview } from "@tauri-apps/api/webview";
|
||||||
@@ -23,6 +23,7 @@ import {
|
|||||||
sanitizeRelayUrl,
|
sanitizeRelayUrl,
|
||||||
} from "../../lib/urlRelay";
|
} from "../../lib/urlRelay";
|
||||||
import { classifyDrop, DROP_BLOCKED_TOAST } from "../../lib/dropTarget";
|
import { classifyDrop, DROP_BLOCKED_TOAST } from "../../lib/dropTarget";
|
||||||
|
import { useSignInOpenTarget } from "../../hooks/useSignInOpenTarget";
|
||||||
import UrlToast, {
|
import UrlToast, {
|
||||||
URL_TOAST_PRIMARY_SELECTOR,
|
URL_TOAST_PRIMARY_SELECTOR,
|
||||||
URL_TOAST_SELECTOR,
|
URL_TOAST_SELECTOR,
|
||||||
@@ -48,6 +49,22 @@ interface Props {
|
|||||||
*/
|
*/
|
||||||
export type PromptSource = "relay" | UrlSource;
|
export type PromptSource = "relay" | UrlSource;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* What the shared prompt slot holds.
|
||||||
|
*
|
||||||
|
* `seq` is identity: the slot is one long-lived place that several prompts pass
|
||||||
|
* through, so "is this still the prompt I acted on?" cannot be answered by the
|
||||||
|
* URL (the same link can legitimately be relayed twice) and must not be
|
||||||
|
* answered by "is anything there?". It keys the toast for remounting *and*
|
||||||
|
* guards the deferred dismissal — see `dismissUrlPromptIfCurrent`.
|
||||||
|
*/
|
||||||
|
interface UrlPrompt {
|
||||||
|
url: string;
|
||||||
|
label: string;
|
||||||
|
source: PromptSource;
|
||||||
|
seq: number;
|
||||||
|
}
|
||||||
|
|
||||||
/** Higher wins. Provenance, not recency. */
|
/** Higher wins. Provenance, not recency. */
|
||||||
const SOURCE_RANK: Record<PromptSource, number> = {
|
const SOURCE_RANK: Record<PromptSource, number> = {
|
||||||
heuristic: 0,
|
heuristic: 0,
|
||||||
@@ -131,17 +148,22 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
// replacing a first would otherwise mutate the toast in place, swapping the
|
// replacing a first would otherwise mutate the toast in place, swapping the
|
||||||
// text under a user who is mid-read and mid-click. Keying the toast on it
|
// text under a user who is mid-read and mid-click. Keying the toast on it
|
||||||
// remounts the component, so a new URL is unmistakably a new prompt.
|
// remounts the component, so a new URL is unmistakably a new prompt.
|
||||||
const [urlPrompt, setUrlPrompt] = useState<{
|
const [urlPrompt, setUrlPrompt] = useState<UrlPrompt | null>(null);
|
||||||
url: string;
|
|
||||||
label: string;
|
|
||||||
source: PromptSource;
|
|
||||||
seq: number;
|
|
||||||
} | null>(null);
|
|
||||||
const promptSeqRef = useRef(0);
|
const promptSeqRef = useRef(0);
|
||||||
const relayLimiterRef = useRef(new RelayRateLimiter());
|
const relayLimiterRef = useRef(new RelayRateLimiter());
|
||||||
// Read by the long-lived keyboard listener below, which is registered once
|
/**
|
||||||
// and would otherwise close over the prompt as it was at mount.
|
* A mirror of the prompt slot, written *eagerly* by the two functions that
|
||||||
const urlPromptRef = useRef<{ url: string } | null>(null);
|
* change it.
|
||||||
|
*
|
||||||
|
* Read by the long-lived keyboard listener below, which is registered once
|
||||||
|
* and would otherwise close over the prompt as it was at mount — and by
|
||||||
|
* {@link dismissUrlPromptIfCurrent}, which is the reason it is written on the
|
||||||
|
* spot rather than from an effect. An effect-synced mirror lags the state it
|
||||||
|
* mirrors by a commit, and the whole question that identity check answers is
|
||||||
|
* "did a new prompt land while I was awaiting?" — a mirror that has not
|
||||||
|
* caught up yet answers it wrong in exactly the window that matters.
|
||||||
|
*/
|
||||||
|
const urlPromptRef = useRef<UrlPrompt | null>(null);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Empty the prompt slot, and put focus somewhere real if it was inside the
|
* Empty the prompt slot, and put focus somewhere real if it was inside the
|
||||||
@@ -156,10 +178,40 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
*/
|
*/
|
||||||
const dismissUrlPrompt = useCallback(() => {
|
const dismissUrlPrompt = useCallback(() => {
|
||||||
const wasInside = !!document.activeElement?.closest(URL_TOAST_SELECTOR);
|
const wasInside = !!document.activeElement?.closest(URL_TOAST_SELECTOR);
|
||||||
|
urlPromptRef.current = null;
|
||||||
setUrlPrompt(null);
|
setUrlPrompt(null);
|
||||||
if (wasInside) termRef.current?.focus();
|
if (wasInside) termRef.current?.focus();
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Dismiss, but only if the slot is still holding the prompt the caller
|
||||||
|
* acted on.
|
||||||
|
*
|
||||||
|
* For anything that dismisses *after* awaiting. `openUrlExternal` takes at
|
||||||
|
* least `OPENER_GRACE` (400 ms, doubled when `xdg-open` fails and `gio` is
|
||||||
|
* tried) on Linux by construction, and the container can relay a second,
|
||||||
|
* superseding URL inside that window — at which point the slot has been
|
||||||
|
* remounted with prompt B and an unconditional `setUrlPrompt(null)` blanks
|
||||||
|
* it. The user never sees B, and B exists nowhere but the container's
|
||||||
|
* transcript, which is the exact failure "dismiss on success only" was
|
||||||
|
* introduced to prevent.
|
||||||
|
*
|
||||||
|
* This is a sibling of {@link dismissUrlPrompt} rather than an optional
|
||||||
|
* `expectedSeq` parameter on it, because `dismissUrlPrompt` is handed
|
||||||
|
* straight to `onClick`/`onDismiss`: React would call it with a `MouseEvent`
|
||||||
|
* as its first argument, that event would land in `expectedSeq`, and the ✕
|
||||||
|
* button would silently stop dismissing anything. A parameter that is only
|
||||||
|
* ever correct when nobody passes it by reference is not a safe signature
|
||||||
|
* here.
|
||||||
|
*/
|
||||||
|
const dismissUrlPromptIfCurrent = useCallback(
|
||||||
|
(seq: number) => {
|
||||||
|
if (urlPromptRef.current?.seq !== seq) return;
|
||||||
|
dismissUrlPrompt();
|
||||||
|
},
|
||||||
|
[dismissUrlPrompt],
|
||||||
|
);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The only writer of the prompt slot. Re-validates whatever the caller
|
* The only writer of the prompt slot. Re-validates whatever the caller
|
||||||
* found: the OSC relay branch has already been through `parseUrlRelayOsc`,
|
* found: the OSC relay branch has already been through `parseUrlRelayOsc`,
|
||||||
@@ -178,17 +230,19 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
console.warn("Refusing to prompt for a URL that failed validation");
|
console.warn("Refusing to prompt for a URL that failed validation");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
setUrlPrompt((current) => {
|
// Read and written through the ref rather than a functional update, so
|
||||||
if (!supersedes({ url, source }, current)) return current;
|
// the mirror is current the instant this returns. Two prompts arriving in
|
||||||
|
// one tick still see each other — that is what the ref being the eager
|
||||||
|
// copy buys — and the seq counter no longer advances inside a state
|
||||||
|
// updater, which React is free to run twice.
|
||||||
|
if (!supersedes({ url, source }, urlPromptRef.current)) return;
|
||||||
promptSeqRef.current += 1;
|
promptSeqRef.current += 1;
|
||||||
return { url, label, source, seq: promptSeqRef.current };
|
const next: UrlPrompt = { url, label, source, seq: promptSeqRef.current };
|
||||||
});
|
urlPromptRef.current = next;
|
||||||
|
setUrlPrompt(next);
|
||||||
},
|
},
|
||||||
[],
|
[],
|
||||||
);
|
);
|
||||||
useEffect(() => {
|
|
||||||
urlPromptRef.current = urlPrompt;
|
|
||||||
}, [urlPrompt]);
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The keyboard route into the toast.
|
* The keyboard route into the toast.
|
||||||
@@ -409,7 +463,18 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
console.warn("Refusing to open a link that failed validation");
|
console.warn("Refusing to open a link that failed validation");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
openUrl(safe).catch((e) => console.error("Failed to open URL:", e));
|
// Same failure reporting as the toast's Open button — see the long note
|
||||||
|
// on `handleOpenUrl`, including what this catch does *not* catch on
|
||||||
|
// Linux. A click that appears to do nothing is the complaint either way.
|
||||||
|
openUrlExternal(safe).catch((e) =>
|
||||||
|
useAppState.getState().pushToast({
|
||||||
|
kind: "error",
|
||||||
|
message: "Could not open that link in your browser",
|
||||||
|
detail: String(e),
|
||||||
|
// A dead opener fails for every link in the buffer. One card.
|
||||||
|
dedupeKey: "host-open-failed",
|
||||||
|
}),
|
||||||
|
);
|
||||||
}, { urlRegex });
|
}, { urlRegex });
|
||||||
term.loadAddon(webLinksAddon);
|
term.loadAddon(webLinksAddon);
|
||||||
|
|
||||||
@@ -786,19 +851,65 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
return () => clearTimeout(timer);
|
return () => clearTimeout(timer);
|
||||||
}, [imagePasteMsg]);
|
}, [imagePasteMsg]);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Hand the prompted URL to the host's browser.
|
||||||
|
*
|
||||||
|
* Two things here are ordering, not decoration:
|
||||||
|
*
|
||||||
|
* - **The toast is dismissed on success only, and only if it is still the
|
||||||
|
* same toast.** Dismissing first is what this replaced: a failed open left
|
||||||
|
* the user with an empty screen and no way back to a URL that only exists
|
||||||
|
* in the container's transcript. Now a failure keeps the prompt exactly
|
||||||
|
* where it was, which also leaves "In container" one click away — the
|
||||||
|
* fallback this failure is the argument for. Waiting to dismiss opens a
|
||||||
|
* second window, though: the open is awaited, the container can relay a
|
||||||
|
* superseding URL while it is in flight, and blanking the slot on success
|
||||||
|
* would then throw away a prompt the user has never seen. Hence the seq
|
||||||
|
* check in `dismissUrlPromptIfCurrent` rather than a bare dismissal.
|
||||||
|
* - **The failure is a toast, not a `console.error`.** Same `pushToast` the
|
||||||
|
* container-browser branch below uses, because from the user's side the
|
||||||
|
* two actions fail identically: nothing happens.
|
||||||
|
*
|
||||||
|
* What this does *not* cover, and must not be described as covering: on Linux
|
||||||
|
* `xdg-open` routinely exits 0 having done nothing useful, so the most common
|
||||||
|
* Linux failure resolves this promise and reports success. Stripping the
|
||||||
|
* leaked AppImage environment before the browser is spawned is what addresses
|
||||||
|
* that; this is the complement that catches everything which does report.
|
||||||
|
*/
|
||||||
const handleOpenUrl = useCallback(() => {
|
const handleOpenUrl = useCallback(() => {
|
||||||
if (!urlPrompt) return;
|
if (!urlPrompt) return;
|
||||||
// Validated again at the sink. `promptUrl` is the only writer and already
|
// Validated again at the sink. `promptUrl` is the only writer and already
|
||||||
// sanitizes, so this can only fail if that invariant is broken — which is
|
// sanitizes, so this can only fail if that invariant is broken — which is
|
||||||
// precisely when it matters that the last thing before `openUrl` checks.
|
// precisely when it matters that the last thing before the opener checks.
|
||||||
const safe = sanitizeRelayUrl(urlPrompt.url);
|
const safe = sanitizeRelayUrl(urlPrompt.url);
|
||||||
dismissUrlPrompt();
|
|
||||||
if (!safe) {
|
if (!safe) {
|
||||||
console.warn("Refusing to open a URL that failed validation");
|
console.warn("Refusing to open a URL that failed validation");
|
||||||
|
dismissUrlPrompt();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
openUrl(safe).catch((e) => console.error("Failed to open URL:", e));
|
// The prompt this click was for. Captured before the await, because the
|
||||||
}, [urlPrompt, dismissUrlPrompt]);
|
// slot may be holding a different one by the time the opener answers.
|
||||||
|
const openedSeq = urlPrompt.seq;
|
||||||
|
openUrlExternal(safe)
|
||||||
|
.then(() => dismissUrlPromptIfCurrent(openedSeq))
|
||||||
|
.catch((e) =>
|
||||||
|
useAppState.getState().pushToast({
|
||||||
|
kind: "error",
|
||||||
|
message: "Could not open it in your browser",
|
||||||
|
detail: String(e),
|
||||||
|
dedupeKey: "host-open-failed",
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
}, [urlPrompt, dismissUrlPrompt, dismissUrlPromptIfCurrent]);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Which action leads when the prompt is holding an Anthropic sign-in link.
|
||||||
|
*
|
||||||
|
* Resolved per project, not per URL — see `useSignInOpenTarget`. The toast
|
||||||
|
* offers both regardless; this is only which one is filled in and reachable
|
||||||
|
* with {@link URL_TOAST_SHORTCUT}.
|
||||||
|
*/
|
||||||
|
const signInDefault = useSignInOpenTarget(projectId);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Open the prompted URL in the container's own browser instead of the host's.
|
* Open the prompted URL in the container's own browser instead of the host's.
|
||||||
@@ -811,6 +922,13 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
const handleOpenUrlInContainer = useCallback(() => {
|
const handleOpenUrlInContainer = useCallback(() => {
|
||||||
if (!urlPrompt) return;
|
if (!urlPrompt) return;
|
||||||
const safe = sanitizeRelayUrl(urlPrompt.url);
|
const safe = sanitizeRelayUrl(urlPrompt.url);
|
||||||
|
// Unconditional, and it needs no seq guard, because it happens *before* the
|
||||||
|
// first await: nothing else can have touched the slot between the click and
|
||||||
|
// this line. The success and failure reports below are toasts rather than
|
||||||
|
// this prompt coming back, so there is nothing here that has to survive the
|
||||||
|
// round trip — which is what makes dismissing up front correct here and
|
||||||
|
// wrong in `handleOpenUrl`. Anything that moves this dismissal after the
|
||||||
|
// `openPageInContainerBrowser` call has to take the seq with it.
|
||||||
dismissUrlPrompt();
|
dismissUrlPrompt();
|
||||||
if (!safe) {
|
if (!safe) {
|
||||||
console.warn("Refusing to open a URL that failed validation");
|
console.warn("Refusing to open a URL that failed validation");
|
||||||
@@ -896,6 +1014,7 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
label={urlPrompt.label}
|
label={urlPrompt.label}
|
||||||
onOpen={handleOpenUrl}
|
onOpen={handleOpenUrl}
|
||||||
onOpenInContainer={handleOpenUrlInContainer}
|
onOpenInContainer={handleOpenUrlInContainer}
|
||||||
|
signInDefault={signInDefault}
|
||||||
onDismiss={dismissUrlPrompt}
|
onDismiss={dismissUrlPrompt}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -105,6 +105,7 @@ describe("UrlToast", () => {
|
|||||||
url={SIGN_IN}
|
url={SIGN_IN}
|
||||||
onOpen={noop}
|
onOpen={noop}
|
||||||
onOpenInContainer={noop}
|
onOpenInContainer={noop}
|
||||||
|
signInDefault="container"
|
||||||
onDismiss={noop}
|
onDismiss={noop}
|
||||||
/>,
|
/>,
|
||||||
);
|
);
|
||||||
@@ -150,6 +151,7 @@ describe("UrlToast", () => {
|
|||||||
url={SIGN_IN}
|
url={SIGN_IN}
|
||||||
onOpen={noop}
|
onOpen={noop}
|
||||||
onOpenInContainer={noop}
|
onOpenInContainer={noop}
|
||||||
|
signInDefault="container"
|
||||||
onDismiss={noop}
|
onDismiss={noop}
|
||||||
/>,
|
/>,
|
||||||
);
|
);
|
||||||
@@ -166,10 +168,11 @@ describe("UrlToast", () => {
|
|||||||
|
|
||||||
describe("Anthropic sign-in links", () => {
|
describe("Anthropic sign-in links", () => {
|
||||||
// The callback listener a `claude login` is waiting on is *inside* the
|
// The callback listener a `claude login` is waiting on is *inside* the
|
||||||
// container. Sending the user to their host browser completes the sign-in
|
// container, so a sign-in is the one case where the host browser may be the
|
||||||
// and then posts the result where nothing is listening, and the terminal
|
// wrong lead. Whether it actually is depends on the project — a live auth
|
||||||
// hangs to its timeout — so for these, and only these, the container-side
|
// bridge carries the callback back, and the container-side alternative is
|
||||||
// browser leads.
|
// not installed on a fresh project — so the owner decides and passes
|
||||||
|
// `signInDefault`. This component only renders the decision.
|
||||||
const SIGN_IN =
|
const SIGN_IN =
|
||||||
"https://claude.ai/oauth/authorize?code=true&client_id=abc&response_type=code";
|
"https://claude.ai/oauth/authorize?code=true&client_id=abc&response_type=code";
|
||||||
|
|
||||||
@@ -180,7 +183,77 @@ describe("UrlToast", () => {
|
|||||||
.filter((t) => t === "Open" || t === "In container");
|
.filter((t) => t === "Open" || t === "In container");
|
||||||
}
|
}
|
||||||
|
|
||||||
it("puts the container browser first", () => {
|
it("puts the container browser first when the caller asks for it", () => {
|
||||||
|
render(
|
||||||
|
<UrlToast
|
||||||
|
url={SIGN_IN}
|
||||||
|
onOpen={noop}
|
||||||
|
onOpenInContainer={noop}
|
||||||
|
signInDefault="container"
|
||||||
|
onDismiss={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(actions()).toEqual(["In container", "Open"]);
|
||||||
|
expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent(
|
||||||
|
/callback listener is inside the container/i,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leads with the host, and promises the bridge, when the bridge is live", () => {
|
||||||
|
// The pair is unchanged; only the order and which one is filled.
|
||||||
|
render(
|
||||||
|
<UrlToast
|
||||||
|
url={SIGN_IN}
|
||||||
|
onOpen={noop}
|
||||||
|
onOpenInContainer={noop}
|
||||||
|
signInDefault="host-bridged"
|
||||||
|
onDismiss={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(actions()).toEqual(["Open", "In container"]);
|
||||||
|
expect(
|
||||||
|
document.querySelector(URL_TOAST_PRIMARY_SELECTOR),
|
||||||
|
).toHaveTextContent("Open");
|
||||||
|
// Still recognised as a sign-in, so the explanation stays — and here the
|
||||||
|
// explanation is true, which is the only state in which it may be given.
|
||||||
|
expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent(
|
||||||
|
/the auth bridge is what carries the callback/i,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("says the callback has nothing carrying it when the host is the last resort", () => {
|
||||||
|
// `host-fallback`: bridge off or unknown *and* no browser in the
|
||||||
|
// container. The old two-state hint said the auth bridge would carry the
|
||||||
|
// callback here too, which is a false promise — the user opens the link
|
||||||
|
// in their own browser and `claude login` hangs to its timeout with
|
||||||
|
// nothing on screen explaining why.
|
||||||
|
render(
|
||||||
|
<UrlToast
|
||||||
|
url={SIGN_IN}
|
||||||
|
onOpen={noop}
|
||||||
|
onOpenInContainer={noop}
|
||||||
|
signInDefault="host-fallback"
|
||||||
|
onDismiss={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
// Which button leads does not change — only what the hint claims.
|
||||||
|
expect(actions()).toEqual(["Open", "In container"]);
|
||||||
|
expect(
|
||||||
|
document.querySelector(URL_TOAST_PRIMARY_SELECTOR),
|
||||||
|
).toHaveTextContent("Open");
|
||||||
|
const hint = screen.getByTestId("url-toast-signin-hint");
|
||||||
|
expect(hint).toHaveTextContent(/nothing is set up to reach it/i);
|
||||||
|
// And it points at the two things that would fix it, since a warning
|
||||||
|
// with no next step is only a nicer way to fail.
|
||||||
|
expect(hint).toHaveTextContent(/Auth bridge/);
|
||||||
|
expect(hint).toHaveTextContent(/install browser support/i);
|
||||||
|
expect(hint).not.toHaveTextContent(/the auth bridge is what carries the callback/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("defaults to the least-bad reading when the caller passes nothing", () => {
|
||||||
|
// A caller that says nothing has not told us a bridge is live, so the
|
||||||
|
// hint must not invent one. The host still leads: it is the answer more
|
||||||
|
// likely to work, and the one that reports its own failure.
|
||||||
render(
|
render(
|
||||||
<UrlToast
|
<UrlToast
|
||||||
url={SIGN_IN}
|
url={SIGN_IN}
|
||||||
@@ -189,9 +262,9 @@ describe("UrlToast", () => {
|
|||||||
onDismiss={noop}
|
onDismiss={noop}
|
||||||
/>,
|
/>,
|
||||||
);
|
);
|
||||||
expect(actions()).toEqual(["In container", "Open"]);
|
expect(actions()).toEqual(["Open", "In container"]);
|
||||||
expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent(
|
expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent(
|
||||||
/callback listener is inside the container/i,
|
/nothing is set up to reach it/i,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -202,6 +275,7 @@ describe("UrlToast", () => {
|
|||||||
url={SIGN_IN}
|
url={SIGN_IN}
|
||||||
onOpen={onOpen}
|
onOpen={onOpen}
|
||||||
onOpenInContainer={noop}
|
onOpenInContainer={noop}
|
||||||
|
signInDefault="container"
|
||||||
onDismiss={noop}
|
onDismiss={noop}
|
||||||
/>,
|
/>,
|
||||||
);
|
);
|
||||||
@@ -211,12 +285,14 @@ describe("UrlToast", () => {
|
|||||||
|
|
||||||
it("leaves an ordinary URL alone", () => {
|
it("leaves an ordinary URL alone", () => {
|
||||||
// A `gh auth login` device code, a docs page, a preview build — the host
|
// A `gh auth login` device code, a docs page, a preview build — the host
|
||||||
// browser is the right answer for all of them and stays the default.
|
// browser is the right answer for all of them and stays the default,
|
||||||
|
// whatever the project's sign-in preference happens to be.
|
||||||
render(
|
render(
|
||||||
<UrlToast
|
<UrlToast
|
||||||
url="https://github.com/login/device?code=ABCD-EFGH"
|
url="https://github.com/login/device?code=ABCD-EFGH"
|
||||||
onOpen={noop}
|
onOpen={noop}
|
||||||
onOpenInContainer={noop}
|
onOpenInContainer={noop}
|
||||||
|
signInDefault="container"
|
||||||
onDismiss={noop}
|
onDismiss={noop}
|
||||||
/>,
|
/>,
|
||||||
);
|
);
|
||||||
@@ -232,6 +308,7 @@ describe("UrlToast", () => {
|
|||||||
url="https://claude.ai.evil.tld/oauth/authorize?x=1"
|
url="https://claude.ai.evil.tld/oauth/authorize?x=1"
|
||||||
onOpen={noop}
|
onOpen={noop}
|
||||||
onOpenInContainer={noop}
|
onOpenInContainer={noop}
|
||||||
|
signInDefault="container"
|
||||||
onDismiss={noop}
|
onDismiss={noop}
|
||||||
/>,
|
/>,
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import type { KeyboardEvent } from "react";
|
import type { KeyboardEvent } from "react";
|
||||||
import { isAnthropicSignInUrl, urlOrigin } from "../../lib/urlRelay";
|
import { isAnthropicSignInUrl, urlOrigin } from "../../lib/urlRelay";
|
||||||
|
import type { SignInOpenTarget } from "../../hooks/useSignInOpenTarget";
|
||||||
import Button from "../ui/Button";
|
import Button from "../ui/Button";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -37,6 +38,24 @@ interface Props {
|
|||||||
/** Open it in the container's own browser instead of the host's. Omitted when
|
/** Open it in the container's own browser instead of the host's. Omitted when
|
||||||
* the project has no browser to open it in. */
|
* the project has no browser to open it in. */
|
||||||
onOpenInContainer?: () => void;
|
onOpenInContainer?: () => void;
|
||||||
|
/**
|
||||||
|
* Which action leads for a *sign-in* link, and why (see the note below).
|
||||||
|
* Nothing else in the toast moves: both buttons are offered in all three
|
||||||
|
* states, in one of two orders.
|
||||||
|
*
|
||||||
|
* This component does not work it out, because the answer depends on the
|
||||||
|
* project's auth bridge and on what is installed inside its container —
|
||||||
|
* neither of which a presentational component should be reaching for.
|
||||||
|
* `hooks/useSignInOpenTarget.ts` owns the rule.
|
||||||
|
*
|
||||||
|
* Two of the three lead with the host button and differ only in the hint,
|
||||||
|
* which is the whole point of carrying three: `"host-bridged"` may promise
|
||||||
|
* that the auth bridge brings the callback home, `"host-fallback"` may not,
|
||||||
|
* because in that state nothing does. `"host-fallback"` is the default for
|
||||||
|
* that reason — a caller that says nothing has not told us a bridge is live,
|
||||||
|
* and the hint must not invent one.
|
||||||
|
*/
|
||||||
|
signInDefault?: SignInOpenTarget;
|
||||||
onDismiss: () => void;
|
onDismiss: () => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -57,17 +76,26 @@ interface Props {
|
|||||||
* text swaps with no animation, and a user reading URL A can click Open on URL
|
* text swaps with no animation, and a user reading URL A can click Open on URL
|
||||||
* B that arrived a second later.
|
* B that arrived a second later.
|
||||||
*
|
*
|
||||||
* ## Anthropic sign-in links default to the container's browser
|
* ## Anthropic sign-in links get their default from the caller
|
||||||
*
|
*
|
||||||
* For an ordinary URL the host browser is the right answer and stays the
|
* For an ordinary URL the host browser is the right answer and stays the
|
||||||
* default. For a sign-in it is the *wrong* one: the callback listener the CLI
|
* default, unconditionally. A sign-in is the one case where it might not be:
|
||||||
* is waiting on is inside the container, so a host browser completes the sign-in
|
* the callback listener the CLI is waiting on is inside the container, so a
|
||||||
* and then posts the result somewhere nothing is listening, and the terminal
|
* host browser can complete the sign-in and then post the result where nothing
|
||||||
* hangs until it times out. Making the host button primary there was quietly
|
* is listening, leaving the terminal to hang to its timeout.
|
||||||
* steering every user into that. The container-side browser closes the loop
|
*
|
||||||
* with no host round trip and no auth bridge, so it leads — and the host button
|
* *Can*, not *does* — which is why this is no longer decided from the URL. The
|
||||||
* stays, because a user who has the auth bridge on, or who wants their existing
|
* auth bridge mirrors that container listener onto the same host port, and the
|
||||||
* browser session, still needs it.
|
* container-side alternative is Playwright's dashboard pane, which a fresh
|
||||||
|
* project has not installed. Both of those are project facts, so the owner
|
||||||
|
* passes {@link Props.signInDefault} and this only renders it: the leading
|
||||||
|
* button is filled and comes first, the other keeps its place beside it.
|
||||||
|
*
|
||||||
|
* The hint below the URL renders all *three* states, not the two orderings.
|
||||||
|
* "Neither is set up" also leads with the host, but it is not the same claim:
|
||||||
|
* there the callback has nothing carrying it, so the hint names what would fix
|
||||||
|
* that instead of describing a bridge that is off. A two-way hint keyed on
|
||||||
|
* which button leads is exactly how that false promise got shipped.
|
||||||
*
|
*
|
||||||
* ## Reachable without a mouse, and it does not take focus to manage it
|
* ## Reachable without a mouse, and it does not take focus to manage it
|
||||||
*
|
*
|
||||||
@@ -96,6 +124,7 @@ export default function UrlToast({
|
|||||||
label = "Long URL detected",
|
label = "Long URL detected",
|
||||||
onOpen,
|
onOpen,
|
||||||
onOpenInContainer,
|
onOpenInContainer,
|
||||||
|
signInDefault = "host-fallback",
|
||||||
onDismiss,
|
onDismiss,
|
||||||
}: Props) {
|
}: Props) {
|
||||||
const origin = urlOrigin(url);
|
const origin = urlOrigin(url);
|
||||||
@@ -103,18 +132,27 @@ export default function UrlToast({
|
|||||||
// Only when there is somewhere to send it: without `onOpenInContainer` the
|
// Only when there is somewhere to send it: without `onOpenInContainer` the
|
||||||
// host button is the only action there is, so it stays primary.
|
// host button is the only action there is, so it stays primary.
|
||||||
const signIn = !!onOpenInContainer && isAnthropicSignInUrl(url);
|
const signIn = !!onOpenInContainer && isAnthropicSignInUrl(url);
|
||||||
|
// A sign-in link the caller has decided is better completed inside the
|
||||||
|
// container. Everything below keys off this rather than off `signIn`, so the
|
||||||
|
// two orderings differ only in which of the pair leads.
|
||||||
|
const containerLeads = signIn && signInDefault === "container";
|
||||||
|
// The third state. Both host states put the same button first, so this is
|
||||||
|
// read by the hint alone: no bridge and no container browser means nothing is
|
||||||
|
// carrying the callback back, and saying "the auth bridge is what carries it"
|
||||||
|
// here is a promise the project cannot keep.
|
||||||
|
const hostIsLastResort = signIn && signInDefault === "host-fallback";
|
||||||
|
|
||||||
// `Button` already owns the filled/outlined variants — including the rule
|
// `Button` already owns the filled/outlined variants — including the rule
|
||||||
// that filled uses `--accent-emphasis` and never `--accent`, which is the
|
// that filled uses `--accent-emphasis` and never `--accent`, which is the
|
||||||
// foreground/link accent and fails WCAG AA behind white text.
|
// foreground/link accent and fails WCAG AA behind white text.
|
||||||
const hostButton = (
|
const hostButton = (
|
||||||
<Button
|
<Button
|
||||||
variant={signIn ? "secondary" : "primary"}
|
variant={containerLeads ? "secondary" : "primary"}
|
||||||
data-url-toast-primary={signIn ? undefined : "true"}
|
data-url-toast-primary={containerLeads ? undefined : "true"}
|
||||||
onClick={onOpen}
|
onClick={onOpen}
|
||||||
className="flex-shrink-0"
|
className="flex-shrink-0"
|
||||||
title={
|
title={
|
||||||
signIn
|
containerLeads
|
||||||
? "Open in your own browser instead — the callback then has to reach the container by some other route"
|
? "Open in your own browser instead — the callback then has to reach the container by some other route"
|
||||||
: undefined
|
: undefined
|
||||||
}
|
}
|
||||||
@@ -128,8 +166,8 @@ export default function UrlToast({
|
|||||||
// the container's own loopback, which is where the tool waiting for it is
|
// the container's own loopback, which is where the tool waiting for it is
|
||||||
// listening — no host round trip, no auth bridge.
|
// listening — no host round trip, no auth bridge.
|
||||||
<Button
|
<Button
|
||||||
variant={signIn ? "primary" : "secondary"}
|
variant={containerLeads ? "primary" : "secondary"}
|
||||||
data-url-toast-primary={signIn ? "true" : undefined}
|
data-url-toast-primary={containerLeads ? "true" : undefined}
|
||||||
onClick={onOpenInContainer}
|
onClick={onOpenInContainer}
|
||||||
className="flex-shrink-0"
|
className="flex-shrink-0"
|
||||||
title="Open in a browser inside the container, and watch it in the Browser tab"
|
title="Open in a browser inside the container, and watch it in the Browser tab"
|
||||||
@@ -235,14 +273,16 @@ export default function UrlToast({
|
|||||||
lineHeight: 1.35,
|
lineHeight: 1.35,
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
Sign-in link — the callback listener is inside the container.
|
{containerLeads
|
||||||
Opening it there closes the loop; the host browser needs the auth
|
? "Sign-in link — the callback listener is inside the container. Opening it there closes the loop; the host browser needs the auth bridge."
|
||||||
bridge.
|
: hostIsLastResort
|
||||||
|
? "Sign-in link — the callback listener is inside the container and nothing is set up to reach it. Turn on Auth bridge in the project’s Config tab, or install browser support to sign in inside the container."
|
||||||
|
: "Sign-in link — the callback listener is inside the container. The auth bridge is what carries the callback back to it from your own browser."}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{signIn ? (
|
{containerLeads ? (
|
||||||
<>
|
<>
|
||||||
{containerButton}
|
{containerButton}
|
||||||
{hostButton}
|
{hostButton}
|
||||||
|
|||||||
@@ -0,0 +1,206 @@
|
|||||||
|
import { useEffect, useState } from "react";
|
||||||
|
import { listen } from "@tauri-apps/api/event";
|
||||||
|
import {
|
||||||
|
checkBrowserViewSupport,
|
||||||
|
getAuthBridgeStatus,
|
||||||
|
} from "../lib/tauri-commands";
|
||||||
|
import { canOpenPageInContainerBrowser } from "../lib/browserViewSupport";
|
||||||
|
import type {
|
||||||
|
AuthBridgeChangedEvent,
|
||||||
|
AuthBridgeStatus,
|
||||||
|
PlaywrightDetection,
|
||||||
|
} from "../lib/types";
|
||||||
|
|
||||||
|
/** Emitted by `auth_bridge/mod.rs` whenever the port or conflict set changes. */
|
||||||
|
const AUTH_BRIDGE_EVENT = "auth-bridge-changed";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Which of the URL toast's two buttons should lead for a sign-in link — and,
|
||||||
|
* for the host, *why*.
|
||||||
|
*
|
||||||
|
* Three states rather than two because "host" covers two worlds that are not
|
||||||
|
* the same promise to the user:
|
||||||
|
*
|
||||||
|
* - `host-bridged` — the auth bridge is live, so a sign-in completed in the
|
||||||
|
* user's own browser has its callback carried back to the listener inside
|
||||||
|
* the container. The host is genuinely the better answer here.
|
||||||
|
* - `container` — no bridge, but the container has a browser to open, which
|
||||||
|
* closes the loop locally with nothing crossing to the host.
|
||||||
|
* - `host-fallback` — neither. The host is the *least bad* of two answers
|
||||||
|
* that can both fail, and the toast has to say so: a hint claiming the
|
||||||
|
* bridge will carry the callback is a false promise in this state, and the
|
||||||
|
* user's `claude login` hangs to its timeout with nothing explaining why.
|
||||||
|
*
|
||||||
|
* Only `container` changes which button leads; the split between the two host
|
||||||
|
* states exists so the toast's hint can tell the truth. Keep it that way — the
|
||||||
|
* consumer that folds them back together is the bug this replaced.
|
||||||
|
*/
|
||||||
|
export type SignInOpenTarget = "host-bridged" | "container" | "host-fallback";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether the auth bridge can be relied on to catch a callback for this
|
||||||
|
* project.
|
||||||
|
*
|
||||||
|
* Deliberately **not** gated on `active_ports` being non-empty. There is only
|
||||||
|
* something to bridge once the CLI has bound its callback listener, and the
|
||||||
|
* order in which that happens against the URL landing in the transcript is not
|
||||||
|
* ours to control — requiring a port here would make the answer depend on a
|
||||||
|
* race and flip the default button between two otherwise identical sign-ins.
|
||||||
|
* `enabled` is the durable fact: the poller is watching, and it will mirror the
|
||||||
|
* port the moment it appears.
|
||||||
|
*
|
||||||
|
* A conflict is the exception, because it is the one state where the bridge is
|
||||||
|
* on and nevertheless *cannot* catch the callback — the host port it needed was
|
||||||
|
* already taken. That is precisely when the container-side browser is the
|
||||||
|
* better default, so it must not read as live.
|
||||||
|
*/
|
||||||
|
export function authBridgeIsLive(status: AuthBridgeStatus | null): boolean {
|
||||||
|
if (!status || !status.enabled) return false;
|
||||||
|
return status.conflicts.length === 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The rule, as a pure function of the two things it depends on.
|
||||||
|
*
|
||||||
|
* Both host answers land on the same button, for different reasons — and they
|
||||||
|
* are deliberately *not* the same value:
|
||||||
|
*
|
||||||
|
* - With the bridge live (`host-bridged`), the host browser is strictly
|
||||||
|
* better — it is the user's own signed-in profile, and the callback still
|
||||||
|
* reaches the container.
|
||||||
|
* - With neither available (`host-fallback`), the host is the *more likely to
|
||||||
|
* work* of two imperfect answers, and it is the one that reports its own
|
||||||
|
* failure (see `handleOpenUrl` in `TerminalView`). The container-side target
|
||||||
|
* is Playwright's dashboard pane, and Playwright's browsers are not baked
|
||||||
|
* into the image, so on a fresh project pointing there fails on every
|
||||||
|
* platform after a several-second wait. Nothing carries the callback back in
|
||||||
|
* this state, so the toast says so rather than promising the bridge.
|
||||||
|
*
|
||||||
|
* Whichever way it goes, both buttons stay in the toast. This chooses which one
|
||||||
|
* leads, never which ones exist.
|
||||||
|
*/
|
||||||
|
export function chooseSignInTarget(
|
||||||
|
bridge: AuthBridgeStatus | null,
|
||||||
|
detection: PlaywrightDetection | null,
|
||||||
|
): SignInOpenTarget {
|
||||||
|
if (authBridgeIsLive(bridge)) return "host-bridged";
|
||||||
|
if (canOpenPageInContainerBrowser(detection)) return "container";
|
||||||
|
return "host-fallback";
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* How long a Playwright probe is reused for.
|
||||||
|
*
|
||||||
|
* `check_browser_view_support` is a `docker exec` running a Node probe, and
|
||||||
|
* every terminal tab of a project would otherwise run its own on mount. Five
|
||||||
|
* minutes is long enough that opening a handful of tabs costs one exec, and
|
||||||
|
* short enough that pressing "Set up Playwright" in the Browser tab is
|
||||||
|
* reflected in the default before the user has finished reading the result.
|
||||||
|
*/
|
||||||
|
const DETECTION_TTL_MS = 5 * 60_000;
|
||||||
|
|
||||||
|
const detectionCache = new Map<
|
||||||
|
string,
|
||||||
|
{ at: number; probe: Promise<PlaywrightDetection | null> }
|
||||||
|
>();
|
||||||
|
|
||||||
|
/** The shared, rate-limited probe. Never rejects — "didn't answer" is `null`. */
|
||||||
|
function probeBrowserSupport(projectId: string): Promise<PlaywrightDetection | null> {
|
||||||
|
const hit = detectionCache.get(projectId);
|
||||||
|
if (hit && Date.now() - hit.at < DETECTION_TTL_MS) return hit.probe;
|
||||||
|
const probe = checkBrowserViewSupport(projectId).catch(() => {
|
||||||
|
// A failure is usually a stopped container, which is a state the user
|
||||||
|
// leaves — so it is not worth remembering for five minutes.
|
||||||
|
detectionCache.delete(projectId);
|
||||||
|
return null;
|
||||||
|
});
|
||||||
|
detectionCache.set(projectId, { at: Date.now(), probe });
|
||||||
|
return probe;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Test seam: drops the memoized probes so a case starts from nothing. */
|
||||||
|
export function resetBrowserSupportCache(): void {
|
||||||
|
detectionCache.clear();
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolve the default action for Anthropic sign-in links in this project.
|
||||||
|
*
|
||||||
|
* Resolved at mount rather than when a URL arrives, on purpose: the toast has
|
||||||
|
* two buttons side by side, and a default that settles a second after the
|
||||||
|
* toast appears moves them under a mouse that is already travelling.
|
||||||
|
*
|
||||||
|
* The expensive half is only paid when it can change the answer. The bridge
|
||||||
|
* status is host-side and cheap; the Playwright probe is a container exec, and
|
||||||
|
* a live bridge decides the question before it is ever asked — which, with the
|
||||||
|
* bridge now on by default, is the ordinary case.
|
||||||
|
*/
|
||||||
|
export function useSignInOpenTarget(projectId: string | undefined): SignInOpenTarget {
|
||||||
|
// `host-fallback` is the honest starting point, not `host-bridged`: before
|
||||||
|
// the status call answers, nothing is known to be carrying the callback, and
|
||||||
|
// the hint that claims one is the failure this three-state answer exists to
|
||||||
|
// prevent. Over-warning for the moment before the answer arrives costs a line
|
||||||
|
// of hedged text; under-warning costs a login that hangs to its timeout.
|
||||||
|
const [target, setTarget] = useState<SignInOpenTarget>("host-fallback");
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
if (!projectId) {
|
||||||
|
setTarget("host-fallback");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let cancelled = false;
|
||||||
|
let bridge: AuthBridgeStatus | null = null;
|
||||||
|
let detection: PlaywrightDetection | null = null;
|
||||||
|
|
||||||
|
const settle = () => {
|
||||||
|
if (!cancelled) setTarget(chooseSignInTarget(bridge, detection));
|
||||||
|
};
|
||||||
|
|
||||||
|
const consider = (next: AuthBridgeStatus) => {
|
||||||
|
bridge = next;
|
||||||
|
settle();
|
||||||
|
// Only now is the container's side of it worth an exec.
|
||||||
|
if (authBridgeIsLive(bridge)) return;
|
||||||
|
probeBrowserSupport(projectId).then((d) => {
|
||||||
|
if (cancelled) return;
|
||||||
|
detection = d;
|
||||||
|
settle();
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
getAuthBridgeStatus(projectId)
|
||||||
|
.then((s) => {
|
||||||
|
if (!cancelled) consider(s);
|
||||||
|
})
|
||||||
|
// Nothing to say to the user here: an unanswered status call is fed
|
||||||
|
// through as a bridge that is off, which lands on `container` or
|
||||||
|
// `host-fallback` — and `host-fallback`'s hint is the one that tells the
|
||||||
|
// user the callback has nothing carrying it.
|
||||||
|
.catch(() => {
|
||||||
|
if (!cancelled) consider({ enabled: false, active_ports: [], conflicts: [] });
|
||||||
|
});
|
||||||
|
|
||||||
|
// The switch can be flipped *while a login is hanging* — that is the whole
|
||||||
|
// reason `set_auth_bridge_enabled` exists outside the Config tab's save —
|
||||||
|
// so the default has to follow it rather than reflect whatever was true
|
||||||
|
// when this terminal was opened.
|
||||||
|
let unlisten: (() => void) | undefined;
|
||||||
|
listen<AuthBridgeChangedEvent>(AUTH_BRIDGE_EVENT, (event) => {
|
||||||
|
if (event.payload.project_id !== projectId) return;
|
||||||
|
consider(event.payload.status);
|
||||||
|
})
|
||||||
|
.then((un) => {
|
||||||
|
if (cancelled) un();
|
||||||
|
else unlisten = un;
|
||||||
|
})
|
||||||
|
.catch(() => {});
|
||||||
|
|
||||||
|
return () => {
|
||||||
|
cancelled = true;
|
||||||
|
unlisten?.();
|
||||||
|
};
|
||||||
|
}, [projectId]);
|
||||||
|
|
||||||
|
return target;
|
||||||
|
}
|
||||||
@@ -0,0 +1,53 @@
|
|||||||
|
/**
|
||||||
|
* What a container has to have before anything can be opened *inside* it.
|
||||||
|
*
|
||||||
|
* The Browser tab asks this to decide what to offer; the terminal's URL toast
|
||||||
|
* asks it to decide which of its two buttons should lead. Both need the same
|
||||||
|
* answer, so the predicates live here rather than beside either caller — the
|
||||||
|
* failure this avoids is the toast steering a user at a container-side browser
|
||||||
|
* that the Browser tab is, on the very same screen, offering to install.
|
||||||
|
*
|
||||||
|
* The important thing to know about `PlaywrightDetection` is that browsers are
|
||||||
|
* deliberately **not** baked into the image: the libraries they link against
|
||||||
|
* are, the binaries are a user-pressed install. So "Playwright is present" and
|
||||||
|
* "a page can actually be opened" are two different questions, and a fresh
|
||||||
|
* project answers yes to neither.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import type { PlaywrightDetection } from "./types";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Mirrors Rust `PlaywrightDetection::is_usable` — the packages the live
|
||||||
|
* dashboard needs. Says nothing about whether a browser exists to show in it.
|
||||||
|
*/
|
||||||
|
export function isBrowserViewUsable(d: PlaywrightDetection | null): boolean {
|
||||||
|
return d !== null && d.playwright_version !== null && d.has_bind && d.cli_entry !== null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether `openPageInContainerBrowser` has a browser to launch.
|
||||||
|
*
|
||||||
|
* Stricter than {@link isBrowserViewUsable} on purpose: the packages can be
|
||||||
|
* installed with `~/.cache/ms-playwright` still empty, which is exactly the
|
||||||
|
* state a `playwright install` step exists to leave behind, and launching into
|
||||||
|
* it fails several seconds after the click.
|
||||||
|
*
|
||||||
|
* Unknown reads as "no". A probe that could not run (stopped container, an
|
||||||
|
* image predating these fields) leaves the executable fields absent, and the
|
||||||
|
* caller's fallback — the host browser — is the one that at least reports its
|
||||||
|
* own failure. Over-refusing costs a user one extra click on a button that is
|
||||||
|
* still right there; over-accepting costs them a sign-in that goes nowhere.
|
||||||
|
*/
|
||||||
|
export function canOpenPageInContainerBrowser(d: PlaywrightDetection | null): boolean {
|
||||||
|
if (!isBrowserViewUsable(d) || !d) return false;
|
||||||
|
// The viewer's own Chromium, confirmed on disk by the probe.
|
||||||
|
if (d.chromium_executable_exists) return true;
|
||||||
|
// Google Chrome is an apt package, so it is never in `browsers` and has no
|
||||||
|
// revision to skew against.
|
||||||
|
if (d.chrome_channel !== null) return true;
|
||||||
|
// `== null`, not `=== null`: a probe from a container predating the
|
||||||
|
// executable fields omits them entirely, and `undefined` there means "didn't
|
||||||
|
// answer", not "missing". In that case a non-empty bundle list is the only
|
||||||
|
// evidence available, and it is better than nothing.
|
||||||
|
return d.chromium_executable == null && d.browsers.length > 0;
|
||||||
|
}
|
||||||
@@ -350,8 +350,8 @@ export const sweepClaudeTokenSnapshots = () =>
|
|||||||
// without deleting its volumes. Reset is the destructive alternative: it wipes
|
// without deleting its volumes. Reset is the destructive alternative: it wipes
|
||||||
// ~/.claude, the OAuth credential, installed skills and every transcript.
|
// ~/.claude, the OAuth credential, installed skills and every transcript.
|
||||||
//
|
//
|
||||||
// Flow: getContainerStaleness (read-only, ~6s — two filesystem probes, so call
|
// Flow: getContainerStaleness (~6s — two filesystem probes, so call it on demand
|
||||||
// it on demand rather than polling) → migrateProjectToBase → the project sits
|
// rather than polling) → migrateProjectToBase → the project sits
|
||||||
// in "awaiting-confirmation" while the user tries it → confirmMigration or
|
// in "awaiting-confirmation" while the user tries it → confirmMigration or
|
||||||
// rollbackMigration.
|
// rollbackMigration.
|
||||||
//
|
//
|
||||||
@@ -361,7 +361,19 @@ export const sweepClaudeTokenSnapshots = () =>
|
|||||||
//
|
//
|
||||||
// Progress arrives on the existing `container-progress` event.
|
// Progress arrives on the existing `container-progress` event.
|
||||||
|
|
||||||
/** Read-only. Runs two container/image filesystem probes; not for polling. */
|
/**
|
||||||
|
* Runs two container/image filesystem probes; not for polling.
|
||||||
|
*
|
||||||
|
* **Not read-only, despite only reporting.** When the container is *stopped*
|
||||||
|
* the backend has to commit its writable layer to a throwaway image before it
|
||||||
|
* can read anything — `docker exec` needs a running container — so this writes
|
||||||
|
* (and then removes) an image. The result is cached per stop, so repeat calls
|
||||||
|
* while the container stays stopped are cheap, but the first one after each stop
|
||||||
|
* pays for a commit of the whole layer: seconds on a small project, tens of
|
||||||
|
* seconds on a large one. Do not add a caller that fires more often than "the
|
||||||
|
* container settled into a new state" without re-reading
|
||||||
|
* `get_container_staleness`'s doc comment first.
|
||||||
|
*/
|
||||||
export const getContainerStaleness = (projectId: string) =>
|
export const getContainerStaleness = (projectId: string) =>
|
||||||
invoke<ContainerStaleness>("get_container_staleness", { projectId });
|
invoke<ContainerStaleness>("get_container_staleness", { projectId });
|
||||||
|
|
||||||
@@ -386,3 +398,18 @@ export const rollbackMigration = (projectId: string) =>
|
|||||||
* app crash shows up here as phase "interrupted". */
|
* app crash shows up here as phase "interrupted". */
|
||||||
export const getMigrationState = (projectId: string) =>
|
export const getMigrationState = (projectId: string) =>
|
||||||
invoke<MigrationState | null>("get_migration_state", { projectId });
|
invoke<MigrationState | null>("get_migration_state", { projectId });
|
||||||
|
|
||||||
|
/** Open a URL in the user's own browser.
|
||||||
|
*
|
||||||
|
* Replaces `openUrl` from `@tauri-apps/plugin-opener` at every call site. On
|
||||||
|
* Linux the app ships as an AppImage whose environment leaks into everything
|
||||||
|
* it spawns, which kills a *cold-launched* browser before it paints while
|
||||||
|
* `xdg-open` still exits 0 — so the plugin path reported success and did
|
||||||
|
* nothing (triple-c#34). The Rust side hands the child a repaired environment
|
||||||
|
* and re-validates the URL, which matters because these URLs originate in an
|
||||||
|
* untrusted container. macOS and Windows still reach the plugin, just from
|
||||||
|
* Rust, so there is no platform branch here.
|
||||||
|
*
|
||||||
|
* Rejects with a string already phrased for a toast. */
|
||||||
|
export const openUrlExternal = (url: string) =>
|
||||||
|
invoke<void>("open_url_external", { url });
|
||||||
|
|||||||
@@ -109,7 +109,8 @@ export type UrlCallback = (url: string, source: UrlSource) => void;
|
|||||||
* A direct port of `usable_sign_in_link` in
|
* A direct port of `usable_sign_in_link` in
|
||||||
* `commands/auth_token_commands.rs`, and deliberately just as shallow: this is
|
* `commands/auth_token_commands.rs`, and deliberately just as shallow: this is
|
||||||
* a junk filter, not the security decision. `sanitizeRelayUrl` is still the
|
* a junk filter, not the security decision. `sanitizeRelayUrl` is still the
|
||||||
* only thing standing between any of this and `openUrl`, and duplicating its
|
* only thing standing between any of this and `openUrlExternal`, and
|
||||||
|
* duplicating its
|
||||||
* rules here would be a second place for them to go stale.
|
* rules here would be a second place for them to go stale.
|
||||||
*
|
*
|
||||||
* The one rule from the Rust that is not ported is its `sk-ant-` check: that
|
* The one rule from the Rust that is not ported is its `sk-ant-` check: that
|
||||||
@@ -293,7 +294,7 @@ export class UrlDetector {
|
|||||||
// include the *whole* C0 range and DEL, not just BEL: an escape or a NUL
|
// include the *whole* C0 range and DEL, not just BEL: an escape or a NUL
|
||||||
// swallowed into the middle of a match becomes a URL that renders as one
|
// swallowed into the middle of a match becomes a URL that renders as one
|
||||||
// thing in the toast and resolves as another. Everything emitted here is
|
// thing in the toast and resolves as another. Everything emitted here is
|
||||||
// still re-validated by `sanitizeRelayUrl` before it can reach `openUrl`;
|
// still re-validated by `sanitizeRelayUrl` before it can reach the opener;
|
||||||
// stopping the match early only means the legitimate prefix survives
|
// stopping the match early only means the legitimate prefix survives
|
||||||
// instead of the whole candidate being thrown away.
|
// instead of the whole candidate being thrown away.
|
||||||
// eslint-disable-next-line no-control-regex
|
// eslint-disable-next-line no-control-regex
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import {
|
|||||||
MAX_RELAY_URL_LENGTH,
|
MAX_RELAY_URL_LENGTH,
|
||||||
RelayRateLimiter,
|
RelayRateLimiter,
|
||||||
URL_RELAY_OSC,
|
URL_RELAY_OSC,
|
||||||
|
isAnthropicSignInUrl,
|
||||||
parseUrlRelayOsc,
|
parseUrlRelayOsc,
|
||||||
sanitizeRelayUrl,
|
sanitizeRelayUrl,
|
||||||
urlOrigin,
|
urlOrigin,
|
||||||
@@ -321,3 +322,53 @@ describe("RelayRateLimiter", () => {
|
|||||||
expect(rl.allow("https://c.example/", 10_200)).toBe(true);
|
expect(rl.allow("https://c.example/", 10_200)).toBe(true);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("isAnthropicSignInUrl", () => {
|
||||||
|
// Classification only. Where a sign-in link should be opened is decided by
|
||||||
|
// `hooks/useSignInOpenTarget.ts`, from facts about the project — this answers
|
||||||
|
// the narrower question of whether it is a sign-in link at all, and it does
|
||||||
|
// so through the same allowlist the sign-in flow itself uses.
|
||||||
|
it("recognises the links `claude setup-token` and `claude login` print", () => {
|
||||||
|
expect(
|
||||||
|
isAnthropicSignInUrl(
|
||||||
|
"https://claude.ai/oauth/authorize?code=true&client_id=abc",
|
||||||
|
),
|
||||||
|
).toBe(true);
|
||||||
|
expect(
|
||||||
|
isAnthropicSignInUrl("https://platform.claude.com/oauth/code/callback?x=1"),
|
||||||
|
).toBe(true);
|
||||||
|
expect(isAnthropicSignInUrl("https://console.anthropic.com/login?x=1")).toBe(
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("is not fooled by a host that merely contains an allowed domain", () => {
|
||||||
|
// The thing the allowlist exists for: `claude.ai.evil.tld` ends with
|
||||||
|
// neither `claude.ai` nor `.claude.ai`.
|
||||||
|
expect(isAnthropicSignInUrl("https://claude.ai.evil.tld/oauth/authorize")).toBe(
|
||||||
|
false,
|
||||||
|
);
|
||||||
|
expect(isAnthropicSignInUrl("https://notclaude.ai/login")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("holds the full validator, not just the host test", () => {
|
||||||
|
// It runs `sanitizeRelayUrl`, so everything that cannot be opened at all
|
||||||
|
// is not a sign-in link either — no separate, weaker copy of the rules.
|
||||||
|
expect(isAnthropicSignInUrl("javascript:claude.ai/login")).toBe(false);
|
||||||
|
expect(isAnthropicSignInUrl("https://claude.ai@evil.tld/login")).toBe(false);
|
||||||
|
expect(isAnthropicSignInUrl("https://claude\nai/login")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not claim every allowlisted URL is a sign-in", () => {
|
||||||
|
expect(isAnthropicSignInUrl("https://claude.ai/chat/abc")).toBe(false);
|
||||||
|
expect(isAnthropicSignInUrl("https://www.anthropic.com/news")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves an ordinary link alone, whatever it says in its path", () => {
|
||||||
|
// A `gh auth login` device code is the common one, and sending it to a
|
||||||
|
// container-side browser would be actively wrong.
|
||||||
|
expect(isAnthropicSignInUrl("https://github.com/login/device?code=A")).toBe(
|
||||||
|
false,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
+18
-6
@@ -1,6 +1,7 @@
|
|||||||
/**
|
/**
|
||||||
* URL relay — host side of `container/triple-c-open` — and the single URL
|
* URL relay — host side of `container/triple-c-open` — and the single URL
|
||||||
* validator every `openUrl` call site in the app is required to go through.
|
* validator every `openUrlExternal` call site in the app is required to go
|
||||||
|
* through.
|
||||||
*
|
*
|
||||||
* A CLI inside the container has no browser. When it wants to open a URL
|
* A CLI inside the container has no browser. When it wants to open a URL
|
||||||
* (`gh auth login`, `aws sso login`, `gcloud auth login`, anything honouring
|
* (`gh auth login`, `aws sso login`, `gcloud auth login`, anything honouring
|
||||||
@@ -182,11 +183,22 @@ export function extendsUrl(next: string, current: string): boolean {
|
|||||||
/**
|
/**
|
||||||
* Whether this is a URL that signs the user in to Anthropic.
|
* Whether this is a URL that signs the user in to Anthropic.
|
||||||
*
|
*
|
||||||
* Used to decide *presentation*, not permission — the toast makes the
|
* Classification only. It answers "is this a sign-in link", never "where should
|
||||||
* container-side browser the default action for these, because the OAuth
|
* it be opened" — that decision moved out to `hooks/useSignInOpenTarget.ts`,
|
||||||
* callback listener is inside the container and the host has nothing to catch
|
* because it depends on things this module has no business knowing: whether the
|
||||||
* it with. It is deliberately the same host allowlist the sign-in flow itself
|
* project's auth bridge is live, and whether a browser is actually installed in
|
||||||
* uses, so the two cannot disagree about what a sign-in link is.
|
* the container. This function stays here because the *rule* it encodes is a
|
||||||
|
* URL rule, and it is deliberately the same host allowlist the sign-in flow
|
||||||
|
* itself uses, so the two cannot disagree about what a sign-in link is.
|
||||||
|
*
|
||||||
|
* It used to carry the default with it — container-side always, on the grounds
|
||||||
|
* that "the OAuth callback listener is inside the container and the host has
|
||||||
|
* nothing to catch it with". Both halves of that are now wrong. The host does
|
||||||
|
* have something to catch it with (the auth bridge mirrors the container's
|
||||||
|
* loopback listener onto the same host port), and the container-side target is
|
||||||
|
* not a general browser but Playwright's dashboard pane, whose browsers are
|
||||||
|
* deliberately not baked into the image — so on a fresh project the default
|
||||||
|
* pointed at something that was not installed, on every platform.
|
||||||
*/
|
*/
|
||||||
export function isAnthropicSignInUrl(url: string): boolean {
|
export function isAnthropicSignInUrl(url: string): boolean {
|
||||||
const safe = sanitizeRelayUrl(url, { allowHosts: ANTHROPIC_SIGN_IN_HOSTS });
|
const safe = sanitizeRelayUrl(url, { allowHosts: ANTHROPIC_SIGN_IN_HOSTS });
|
||||||
|
|||||||
Reference in New Issue
Block a user