diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 47d021b3..25c30213 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -106,6 +106,47 @@ jobs: npm run check npm test + # Core's Rust suite, including the headless-Chrome runtime e2e tests. The + # client bundle has to exist on disk first: rust-embed serves from disk in + # debug builds, so without it the server has no client to hand the headless + # page. + test-rust: + runs-on: ubuntu-latest + steps: + - name: Setup repo + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version-file: ".nvmrc" + + - uses: Swatinem/rust-cache@v2 + with: + key: test-rust + save-if: ${{ github.ref == 'refs/heads/main' }} + + - name: Install dependencies + run: npm ci + + - name: Build client bundle + run: npm run build + + - name: Install Playwright browser (chromium) + run: npx playwright install --with-deps chromium + + # The runtime tests skip themselves when no browser is found, which would + # silently gut this job. Point them (and the servers they spawn) at + # Playwright's chromium explicitly: their skip guard reads CHROMIUM_PATH + # with the same precedence the server does, and refuses to skip at all + # when CI is set — so this job can no longer pass having exercised nothing. + - name: Resolve chromium path + run: | + echo "CHROMIUM_PATH=$(node -e "console.log(require('@playwright/test').chromium.executablePath())")" >> "$GITHUB_ENV" + + - name: Run Rust tests + run: cargo test --workspace --all-features + test-e2e: runs-on: ubuntu-latest steps: @@ -197,7 +238,7 @@ jobs: secrets: inherit release: - needs: [config, build, test-frontend, test-e2e, test-e2e-release] + needs: [config, build, test-frontend, test-rust, test-e2e, test-e2e-release] if: needs.config.outputs.publish == 'true' runs-on: ubuntu-latest permissions: @@ -273,7 +314,7 @@ jobs: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} docker: - needs: [config, build, test-frontend, test-e2e, test-e2e-release] + needs: [config, build, test-frontend, test-rust, test-e2e, test-e2e-release] if: needs.config.outputs.publish == 'true' runs-on: ubuntu-latest permissions: diff --git a/Cargo.lock b/Cargo.lock index 8b561fa1..e9dbaffe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2212,6 +2212,7 @@ dependencies = [ "serde_json", "serde_yaml", "silverbullet-server-common", + "silverbullet-server-runtime-chrome", "tempfile", "uuid", ] diff --git a/bin/sb/Cargo.toml b/bin/sb/Cargo.toml index bed5254e..788d1efe 100644 --- a/bin/sb/Cargo.toml +++ b/bin/sb/Cargo.toml @@ -31,3 +31,11 @@ serde_json = { workspace = true } [dev-dependencies] tempfile = { workspace = true } +# Only for `find_chrome()`, so the runtime e2e test skips cleanly on machines +# without a browser instead of failing. +silverbullet-server-runtime-chrome = { path = "../../server-runtime-chrome" } +# `cookies` is declared here rather than relied on from workspace feature +# unification: `bin/silverbullet` enables it, which is enough for a whole- +# workspace build but leaves `cargo clippy -p sb` without +# `ClientBuilder::cookie_store` (used by the multi-space runtime e2e test). +reqwest = { workspace = true, features = ["cookies"] } diff --git a/bin/sb/tests/runtime_multi_e2e.rs b/bin/sb/tests/runtime_multi_e2e.rs new file mode 100644 index 00000000..9a9459c0 --- /dev/null +++ b/bin/sb/tests/runtime_multi_e2e.rs @@ -0,0 +1,334 @@ +//! End-to-end test for the Runtime API in **multi-space** mode, driven through +//! the real `sb` CLI. +//! +//! Two spaces are created on one authenticated server — one bound at the root +//! prefix, one at `/notes` — each holding a different marker page. The test then +//! evaluates Lua in each space through `sb eval` and asserts each answers with +//! its *own* content. That is the assertion that matters: a shared browser must +//! not collapse two spaces into one client. +//! +//! It also asserts the shared browser is genuinely shared, by requiring the +//! server to log its launch line exactly once across both spaces. +//! +//! Skips cleanly when Chrome is absent or when the sibling `silverbullet` +//! server binary hasn't been built (run `cargo test --workspace`, which builds +//! every bin) — except under `CI`, where either missing prerequisite is a hard +//! failure rather than a silent skip (see `common::chrome_available_or_skip` +//! and `server_bin_or_skip`). + +use std::io::Read; +use std::path::PathBuf; +use std::process::{Child, Command, Stdio}; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; + +const ADMIN_USER: &str = "admin"; +const ADMIN_PASSWORD: &str = "adminpw1"; + +#[path = "../../silverbullet/tests/common/mod.rs"] +mod common; +use common::{chrome_available_or_skip, free_port}; + +/// Wraps the spawned server and kills it (plus the shared Chrome it owns) on +/// drop, so nothing leaks even if an assertion panics. +/// +/// stdout/stderr are drained continuously by background threads into `log`, +/// rather than read once at the end. Two headless clients (root + notes) each +/// forward their full `console.*` output into the server's log at INFO level +/// (see `ChromeConfig::log_console`), which is enough volume to fill the OS +/// pipe buffer within seconds; an unread `Stdio::piped()` handle would then +/// make the server block on its own log write, wedging the whole process — +/// including the HTTP listener — with no exception or panic to show for it. +struct Server { + child: Child, + log: Arc>, + readers: Vec>, +} + +/// Continuously copy `reader` into `log` until EOF/error (the process exited +/// and closed the pipe). +fn drain(mut reader: impl Read, log: Arc>) { + let mut buf = [0u8; 8192]; + loop { + match reader.read(&mut buf) { + Ok(0) | Err(_) => return, + Ok(n) => log + .lock() + .unwrap_or_else(|e| e.into_inner()) + .push_str(&String::from_utf8_lossy(&buf[..n])), + } + } +} + +impl Drop for Server { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +impl Server { + /// Spawn `cmd` with piped stdout/stderr, immediately handing both to + /// dedicated reader threads so the child never blocks on a full pipe. + fn spawn(mut cmd: Command) -> Self { + let mut child = cmd + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn silverbullet"); + let log = Arc::new(Mutex::new(String::new())); + let mut readers = Vec::with_capacity(2); + if let Some(out) = child.stdout.take() { + let log = log.clone(); + readers.push(std::thread::spawn(move || drain(out, log))); + } + if let Some(err) = child.stderr.take() { + let log = log.clone(); + readers.push(std::thread::spawn(move || drain(err, log))); + } + Server { + child, + log, + readers, + } + } + + /// Kill the server and return everything captured. Joining the reader + /// threads (rather than sleeping a guessed amount) blocks exactly until + /// each has drained its pipe to EOF, which follows promptly once `wait()` + /// confirms the process has exited and closed its ends. + fn finish(mut self) -> String { + let _ = self.child.kill(); + let _ = self.child.wait(); + for r in self.readers.drain(..) { + let _ = r.join(); + } + self.log.lock().unwrap_or_else(|e| e.into_inner()).clone() + } +} + +fn sb_bin() -> &'static str { + env!("CARGO_BIN_EXE_sb") +} + +/// The sibling `silverbullet` server binary in the same target dir, if built. +/// +/// `None` means this test cannot run at all. Under `CI` that is a hard failure +/// rather than a silent skip, for the same reason as +/// `common::chrome_available_or_skip`: this job gates the release and docker +/// publishes, and a skipped run reads as a pass. It cannot trigger under the +/// current CI command (`cargo test --workspace --all-features` builds every bin +/// into the probed target dir), which is precisely why a silent skip here would +/// go unnoticed the day somebody narrows that command. +fn server_bin_or_skip() -> Option { + let dir = PathBuf::from(sb_bin()).parent()?.to_path_buf(); + let name = if cfg!(windows) { + "silverbullet.exe" + } else { + "silverbullet" + }; + let p = dir.join(name); + if p.exists() { + return Some(p); + } + if std::env::var("CI").is_ok() { + panic!( + "runtime_multi_e2e: no `silverbullet` server binary at {} — this test drives the \ + real server, so it cannot run without one. Refusing to skip in CI: run the suite as \ + `cargo test --workspace --all-features`, which builds every bin into that directory.", + p.display() + ); + } + eprintln!("skipping runtime_multi_e2e: silverbullet server binary not built"); + None +} + +/// Run `sb` with an isolated config home and capture stdout/stderr/exit code. +fn run_sb(args: &[&str], config_home: &std::path::Path) -> (i32, String, String) { + let out = Command::new(sb_bin()) + .args(args) + .env("XDG_CONFIG_HOME", config_home) + .output() + .expect("spawn sb"); + ( + out.status.code().unwrap_or(-1), + String::from_utf8_lossy(&out.stdout).into_owned(), + String::from_utf8_lossy(&out.stderr).into_owned(), + ) +} + +#[test] +fn runtime_api_serves_two_spaces_from_one_shared_chrome() { + if !chrome_available_or_skip("runtime_multi_e2e") { + return; + } + let Some(server) = server_bin_or_skip() else { + return; + }; + + let root = tempfile::tempdir().unwrap(); + let sb_config = tempfile::tempdir().unwrap(); + let chrome_data = root.path().join(".chrome-data"); + + // Provision the root through the real `setup` subcommand: admin account + + // spaces.json, no first space (both spaces are created over the admin API). + let out = Command::new(&server) + .arg("setup") + .arg(root.path()) + .arg("--admin") + .arg(format!("{ADMIN_USER}:{ADMIN_PASSWORD}")) + .output() + .expect("spawn silverbullet setup"); + assert!( + out.status.success(), + "setup failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + + let port = free_port(); + let mut cmd = Command::new(&server); + cmd.arg(root.path()) + .args(["-p", &port.to_string(), "-L", "127.0.0.1"]) + .env_remove("SB_MULTI_SPACE") + .env_remove("SB_USER") + .env_remove("SB_RUNTIME_API") + .env("SB_DISABLE_SERVICE_WORKER", "1") + .env("SB_CHROME_DATA_DIR", &chrome_data); + let server_proc = Server::spawn(cmd); + + let base = format!("http://127.0.0.1:{port}"); + + // Wait for the multi stack: the admin API answers 401 once it's mounted. + let probe = reqwest::blocking::Client::new(); + let deadline = Instant::now() + Duration::from_secs(30); + loop { + if let Ok(r) = probe.get(format!("{base}/.spaces/api/admin/spaces")).send() { + if r.status().as_u16() == 401 { + break; + } + } + assert!(Instant::now() < deadline, "server did not boot in time"); + std::thread::sleep(Duration::from_millis(200)); + } + + // Log in as the admin. + let admin = reqwest::blocking::Client::builder() + .cookie_store(true) + .build() + .unwrap(); + let r = admin + .post(format!("{base}/.spaces/api/login")) + .json(&serde_json::json!({ "username": ADMIN_USER, "password": ADMIN_PASSWORD })) + .send() + .unwrap(); + assert!(r.status().is_success(), "admin login failed"); + + // Two spaces with the runtime API on. One is bound at the ROOT prefix, so + // its auth cookie is set with Path=/ and rides along on the other space's + // requests — exactly the case per-space cookie names exist to handle. + for (name, prefix, folder) in [ + ("Root", "/", "spaceRoot"), + ("Notes", "/notes", "spaceNotes"), + ] { + let r = admin + .post(format!("{base}/.spaces/api/admin/spaces")) + .json(&serde_json::json!({ + "name": name, + "folder": folder, + "binding": { "prefix": prefix }, + "runtimeApi": true + })) + .send() + .unwrap(); + assert!( + r.status().is_success(), + "creating space {name} failed: {}", + r.text().unwrap_or_default() + ); + } + + // A distinguishing page per space, written before any runtime call so the + // headless client picks it up on its initial sync. + std::fs::write( + root.path().join("spaceRoot").join("Marker.md"), + "marker-from-root-space\n", + ) + .unwrap(); + std::fs::write( + root.path().join("spaceNotes").join("Marker.md"), + "marker-from-notes-space\n", + ) + .unwrap(); + + // An admin API token for `sb --token`. + let r = admin + .post(format!( + "{base}/.spaces/api/admin/users/{ADMIN_USER}/tokens" + )) + .json(&serde_json::json!({ "name": "e2e" })) + .send() + .unwrap(); + assert!(r.status().is_success(), "token creation failed"); + let token = r.json::().unwrap()["token"] + .as_str() + .expect("token in response") + .to_string(); + + // Anonymous `sb` must be rejected. + let (code, _, stderr) = run_sb(&["--url", &base, "eval", "1 + 1"], sb_config.path()); + assert_ne!(code, 0, "anonymous eval should fail"); + assert!( + stderr.contains("401") + || stderr.to_lowercase().contains("unauthor") + || stderr.contains("authentication required"), + "expected an auth error, got: {stderr}" + ); + + // Each space answers with its own marker. The first call also pays for the + // browser launch and the client's first sync, so allow a generous window. + for (prefix, expected) in [ + ("", "marker-from-root-space"), + ("/notes", "marker-from-notes-space"), + ] { + let url = format!("{base}{prefix}"); + // Must stay comfortably above the pool's LAUNCH_TIMEOUT (120s in + // server-runtime-chrome/src/pool.rs): the first space's eval pays for + // the browser launch plus a full client boot and sync. + let deadline = Instant::now() + Duration::from_secs(180); + #[allow(unused_assignments)] + let mut last = String::new(); + loop { + let (code, stdout, stderr) = run_sb( + &[ + "--url", + &url, + "--token", + &token, + "eval", + "space.readPage(\"Marker\")", + ], + sb_config.path(), + ); + if code == 0 && stdout.contains(expected) { + break; + } + last = format!("code={code} stdout={stdout:?} stderr={stderr:?}"); + if Instant::now() >= deadline { + let log = server_proc.finish(); + panic!( + "space at {url:?} never returned {expected:?}\nlast: {last}\n\ + --- server log ---\n{log}" + ); + } + std::thread::sleep(Duration::from_secs(1)); + } + } + + // One browser served both spaces. + let log = server_proc.finish(); + let launches = log.matches("launching shared headless Chrome").count(); + assert_eq!( + launches, 1, + "expected exactly one shared Chrome launch, saw {launches}\n--- server log ---\n{log}" + ); +} diff --git a/bin/silverbullet/src/multi.rs b/bin/silverbullet/src/multi.rs index 6aaa945b..3f2ec787 100644 --- a/bin/silverbullet/src/multi.rs +++ b/bin/silverbullet/src/multi.rs @@ -10,7 +10,7 @@ use silverbullet_server::multi::access::SessionPolicy; use silverbullet_server::multi::admin_api::{build_admin_api_router, AdminState}; use silverbullet_server::multi::dispatch::build_main_router; use silverbullet_server::multi::instance::{ - AssetFactories, InstanceAuth, InstanceDeps, RuntimeRequest, + AssetFactories, InstanceAuth, InstanceDeps, RuntimeFactory, RuntimeRequest, }; use silverbullet_server::multi::manager::MultiManager; use silverbullet_server::multi::space_index::{build_spaces_router, SpaceIndexState}; @@ -79,7 +79,7 @@ pub async fn build_multi_stack(config: &Config) -> Result<(axum::Router, String) client_bundle: Box::new(|| Box::new(EmbeddedSpace::::new())), base_fs: Box::new(|| Box::new(EmbeddedSpace::::new())), }, - runtime: Box::new(build_space_runtime), + runtime: space_runtime_factory(&root), metrics: metrics.clone(), auth: InstanceAuth::Accounts { users: store.clone(), @@ -185,27 +185,41 @@ pub async fn run_multi(config: Config) -> Result<(), String> { .map_err(|e| format!("server error: {e}")) } -/// Per-space headless-Chrome runtime factory (same construction as the -/// single-space `build_runtime`). -pub(crate) fn build_space_runtime( - req: &RuntimeRequest, -) -> Option> { - let chrome_cfg = silverbullet_server_runtime_chrome::ChromeConfig::from_env( - req.server_url.clone(), - req.headless_token.to_string(), - req.space_folder, - req.read_only, - )?; - let logs = silverbullet_server::runtime::LogBuffer::new(); - match silverbullet_server_runtime_chrome::ChromeTransport::launch(chrome_cfg, logs.clone()) { - Ok(transport) => Some(Box::new(silverbullet_server::runtime::ClientRuntime::new( - transport, logs, - ))), - Err(e) => { - tracing::warn!("runtime disabled for space: could not launch Chrome: {e}"); +/// Build the runtime factory for a server rooted at `server_root`. +/// +/// One `ChromePool` — one Chrome process — serves every space; each space gets +/// its own page, log buffer, and auth cookie. The pool is created eagerly but +/// launches nothing until some space's runtime API is first used. +pub(crate) fn space_runtime_factory(server_root: &std::path::Path) -> RuntimeFactory { + let pool = match silverbullet_server_runtime_chrome::ChromeConfig::from_env(server_root) { + Some(config) => match silverbullet_server_runtime_chrome::ChromePool::new(config) { + Ok(pool) => Some(pool), + Err(e) => { + tracing::warn!("runtime API disabled: could not create the Chrome pool: {e}"); + None + } + }, + None => { + tracing::info!("runtime API disabled (no Chrome found, or SB_RUNTIME_API=0)"); None } - } + }; + Box::new(move |req: &RuntimeRequest| { + let pool = pool.as_ref()?; + if req.read_only { + return None; + } + let page = silverbullet_server_runtime_chrome::SpacePage { + server_url: req.server_url.clone(), + headless_token: req.headless_token.to_string(), + cookie_name: silverbullet_server::auth::headless_cookie_name(req.space_id), + }; + let logs = silverbullet_server::runtime::LogBuffer::new(); + let transport = pool.transport_for(page, logs.clone()); + Some(Box::new(silverbullet_server::runtime::ClientRuntime::new( + transport, logs, + ))) + }) } #[cfg(unix)] diff --git a/bin/silverbullet/src/single.rs b/bin/silverbullet/src/single.rs index b7dc7e4f..b07f050d 100644 --- a/bin/silverbullet/src/single.rs +++ b/bin/silverbullet/src/single.rs @@ -119,7 +119,7 @@ pub async fn run_single(config: Config) -> Result<(), String> { client_bundle: Box::new(|| Box::new(EmbeddedSpace::::new())), base_fs: Box::new(|| Box::new(EmbeddedSpace::::new())), }, - runtime: Box::new(crate::multi::build_space_runtime), + runtime: crate::multi::space_runtime_factory(&root), metrics: metrics.clone(), auth: InstanceAuth::Single(auth), version: crate::VERSION.to_string(), diff --git a/bin/silverbullet/tests/common/mod.rs b/bin/silverbullet/tests/common/mod.rs index 14d2ce16..61cbcc0c 100644 --- a/bin/silverbullet/tests/common/mod.rs +++ b/bin/silverbullet/tests/common/mod.rs @@ -40,3 +40,40 @@ pub fn free_port() -> u16 { } panic!("no unused port found after 100 attempts"); } + +/// Guard for the headless-Chrome runtime e2e tests: `true` when the server +/// they spawn will be able to find a browser, `false` when the test should skip. +/// +/// The precedence deliberately mirrors `ChromeConfig::from_env` — +/// `SB_CHROME_PATH`, then `CHROMIUM_PATH`, then auto-detection — because that is +/// what the *spawned server* uses. A guard that consults only `find_chrome()` +/// answers a different question: it would skip on a machine where +/// `CHROMIUM_PATH` points at a perfectly good browser (Playwright installs into +/// `~/.cache/ms-playwright/…`, which `find_chrome` does not probe), so pointing +/// CI at Playwright's chromium would not actually stop these tests skipping. +/// +/// And in CI a skip is fatal, not a courtesy: `release` and `docker` gate on +/// this job, so a run that skips its way to green would ship code that nothing +/// exercised. Locally, skipping is still the right behaviour — not every +/// developer has Chrome. +/// +/// Not used by every test binary that includes this module, hence the `allow`. +#[allow(dead_code)] +pub fn chrome_available_or_skip(test_name: &str) -> bool { + let env = |k: &str| std::env::var(k).ok().filter(|v| !v.is_empty()); + if env("SB_CHROME_PATH").is_some() + || env("CHROMIUM_PATH").is_some() + || silverbullet_server_runtime_chrome::find_chrome().is_some() + { + return true; + } + if std::env::var("CI").is_ok() { + panic!( + "{test_name}: no Chrome/Chromium found — SB_CHROME_PATH, CHROMIUM_PATH and \ + auto-detection all came up empty. Refusing to skip in CI: this job gates the \ + release and docker publishes, so a skipped run would ship untested code." + ); + } + eprintln!("skipping {test_name}: no Chrome/Chromium found on this machine"); + false +} diff --git a/bin/silverbullet/tests/runtime_e2e.rs b/bin/silverbullet/tests/runtime_e2e.rs index 9b175881..b6a6145f 100644 --- a/bin/silverbullet/tests/runtime_e2e.rs +++ b/bin/silverbullet/tests/runtime_e2e.rs @@ -3,7 +3,9 @@ //! Spawns the compiled `silverbullet` binary as a subprocess (so killing the //! child cleanly tears down the embedded Chrome), boots it with the runtime //! enabled, and drives `/.runtime/*` over HTTP. Gated on Chrome being available -//! so machines without Chrome skip it cleanly. +//! so machines without Chrome skip it cleanly — except under `CI`, where a +//! missing browser fails the test instead (see +//! `common::chrome_available_or_skip`). use std::io::Read; use std::process::{Child, Command, Stdio}; @@ -21,7 +23,7 @@ impl Drop for Server { } mod common; -use common::free_port; +use common::{chrome_available_or_skip, free_port}; /// Poll `cond` until it returns true or the deadline passes. On timeout, dump the /// server's captured stdout/stderr and panic with `msg`. @@ -58,8 +60,7 @@ fn dump_and_panic(server: &mut Server, msg: &str) -> ! { #[test] fn runtime_api_evaluates_lua_against_headless_chrome() { - if silverbullet_server_runtime_chrome::find_chrome().is_none() { - eprintln!("skipping runtime_e2e: no Chrome/Chromium found on this machine"); + if !chrome_available_or_skip("runtime_e2e") { return; } diff --git a/docs/Architecture/Runtime Manager.md b/docs/Architecture/Runtime Manager.md index bd5804c1..44112d10 100644 --- a/docs/Architecture/Runtime Manager.md +++ b/docs/Architecture/Runtime Manager.md @@ -4,7 +4,7 @@ partOf: "[[Architecture/Server]]" consumes: "[[Architecture/Space Files]]" references: - server-runtime-chrome/src/supervisor.rs -- server-runtime-chrome/src/transport.rs +- server-runtime-chrome/src/pool.rs - server-runtime-chrome/src/lib.rs --- In order to implement the [[Runtime API]], diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 0d1e3b3e..bb615266 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -5,6 +5,17 @@ Whenever a commit is pushed to the `main` branch, within ~5 minutes, it will be * Fix: the FreeBSD **server** binary is being built and released again * Fix: [[Space Manager|multi-space]] mode silently ignored `SB_REMEMBER_ME_HOURS`, `SB_LOCKOUT_TIME`, and `SB_LOCKOUT_LIMIT`, hardcoding “remember me” sessions to 7 days and lockout to 10 attempts per minute. All three now apply there too — server-wide, like the session itself — matching what [[Install/Configuration]] documents. +* Multi-space servers now share a single headless Chrome across all spaces + instead of launching one browser per space. Startup stays lazy: no browser + until the Runtime API is first used, and no tab for a space nobody queries. +* The headless Chrome profile now lives at `/.chrome-data`. + Single-space servers are unaffected (the root *is* the space folder); on a + multi-space server the old per-space `.chrome-data` directories are no longer + used and can be deleted. +* Fix: the Runtime API failed to start when authentication was enabled — the + headless page authorized only its first request and was then redirected to + `/.auth`, leaving `/.runtime/*` answering `bridge_unavailable`. It now + authenticates with a session cookie for the whole session. ([#2072](https://github.com/silverbulletmd/silverbullet/pull/2072)) ## 2.10.0 * [[Space Manager]]: multi-space hosting with multiple accounts is here. A fresh install pointed at an empty folder opens a browser-based first-run **setup wizard** that creates an admin account and your first space, then serves it in place with no restart. One server can host any number of [[Space|spaces]], each bound to a URL prefix or hostname. diff --git a/docs/Install/Configuration.md b/docs/Install/Configuration.md index c5ed48a5..59996395 100644 --- a/docs/Install/Configuration.md +++ b/docs/Install/Configuration.md @@ -44,7 +44,7 @@ To force the classic single-space server on an empty folder, pass `--single` (or * `SB_RUNTIME_API`: The [[Runtime API]] is enabled automatically when Chrome/Chromium is detected on the system. Set to `0` to explicitly disable. Not available in read-only mode. * `SB_CHROME_PATH`: Optional explicit path to the Chrome/Chromium binary. Falls back to the `CHROMIUM_PATH` environment variable (pre-set in the `-runtime-api` Docker image), then auto-detection. * `SB_CHROME_SHOW`: Set to any non-empty value to run Chrome with a visible window instead of headless (useful for debugging). -* `SB_CHROME_DATA_DIR`: Path to persist the Chrome user profile between restarts. When not set, defaults to `.chrome-data` inside the space folder. +* `SB_CHROME_DATA_DIR`: Path to persist the Chrome user profile between restarts. When not set, defaults to `.chrome-data` inside the server root — one profile for the whole server, since one browser serves every space. For a single-space server this is unchanged, because there the root *is* the space folder. * `SB_CHROME_LOG_CONSOLE`: Forward the headless Chrome page’s `console.*` output to the server log (so you can see what the runtime is doing). Enabled by default; set to `0` to disable. The same log is also available via `/.runtime/logs` (e.g. `sb logs`). # Security diff --git a/docs/Runtime API.md b/docs/Runtime API.md index 131cb33a..500321e4 100644 --- a/docs/Runtime API.md +++ b/docs/Runtime API.md @@ -133,7 +133,7 @@ Status codes used across the Runtime API: # How it works As documented in [[Architecture]], the vast majority of SilverBullet’s power is implemented in the client. However, there are use cases for programmatically accessing your space with all of SilverBullet (client’s) power. -When the Runtime API is enabled, the server launches a headless (invisible by default) Chrome process upon the first request to an `/.remote` endpoint. This browser loads the full SilverBullet client, exactly like a regular browser tab, but without a visible window (with some memory optimizations). The client boots normally: it loads all plugs, Lua code and navigates to the index page. +When the Runtime API is enabled, the server launches a single, server-wide headless (invisible by default) Chrome process upon the first request to an `/.remote` endpoint. Each space with the runtime API enabled gets its own page (tab) in that shared browser, opened on that space’s first request. A page loads the full SilverBullet client, exactly like a regular browser tab, but without a visible window (with some memory optimizations). The client boots normally: it loads all plugs, Lua code and navigates to the index page. Once ready, the server communicates with the browser directly via Chrome DevTools Protocol (CDP). Because Lua code runs inside a real SilverBullet client, it has access to the full API surface — `editor.*`, `space.*`, queries, and everything else available to in-page scripts and widgets. The results reflect live client state. @@ -141,4 +141,4 @@ Once ready, the server communicates with the browser directly via Chrome DevTool Set `SB_CHROME_SHOW=1` to run Chrome with a visible window — useful for watching what the headless client is doing. Set `SB_CHROME_DATA_DIR` to a path to persist the Chrome profile between restarts (avoids re-indexing on each restart). ## Resource usage -Headless Chrome spawns several processes (browser, network, storage, and renderer). With the full SilverBullet client loaded and indexed, expect roughly **150–200 MB** of total RSS across all Chrome processes. The SilverBullet server itself adds ~30 MB on top of this. +Headless Chrome spawns several processes (browser, network, storage, and renderer). With the full SilverBullet client loaded and indexed in a single space, expect roughly **150–200 MB** of total RSS across all Chrome processes. Because the browser is shared, additional spaces cost a renderer each rather than a whole browser. The SilverBullet server itself adds ~30 MB on top of this. diff --git a/docs/Space Manager.md b/docs/Space Manager.md index cd5064a0..94ee442a 100644 --- a/docs/Space Manager.md +++ b/docs/Space Manager.md @@ -100,4 +100,4 @@ Single-space mode is the “classic” SilverBullet server: one folder, one spac * Spaces share one OS process and user. This mode is built for a household or team of trusted spaces, not hostile multi-tenancy. * Authentication is shared across the server, while authorization remains per space. Password changes and account deletion revoke that user's sessions immediately; membership and admin-role changes also take effect on the next request. * Because the session is server-wide, so is its policy: `SB_REMEMBER_ME_HOURS`, `SB_LOCKOUT_TIME`, and `SB_LOCKOUT_LIMIT` (see [[Install/Configuration#Authentication]]) apply to every space and to the space list itself, and are set as environment variables rather than per space in `spaces.json`. -* The runtime API (`runtimeApi`) launches one headless Chrome per enabled space, lazily; it is off by default. +* The runtime API (`runtimeApi`) uses a single, server-wide headless Chrome with one page (tab) per enabled space. Both levels are lazy: the browser only starts on the first runtime request from any space, and a space only gets a tab on its own first request. It is off by default. diff --git a/server-runtime-chrome/src/config.rs b/server-runtime-chrome/src/config.rs index b27e25c0..999666b2 100644 --- a/server-runtime-chrome/src/config.rs +++ b/server-runtime-chrome/src/config.rs @@ -1,43 +1,28 @@ use std::path::Path; -/// Resolved configuration for launching the headless browser. +/// Configuration for the single, server-wide headless browser. Resolved once at +/// startup and shared by every space's page. #[derive(Debug, Clone)] pub struct ChromeConfig { pub chrome_path: String, - pub server_url: String, - /// Headless auth token, always appended to the page URL as `&token=…`. When - /// authentication is disabled the open server ignores it; when enabled, the - /// headless-token authorizer accepts it so the headless page is authorized. - pub headless_token: String, pub user_data_dir: String, pub show: bool, - /// Forward the headless page's captured `console.*` output to the server's - /// `tracing` log (under the `runtime_console` target) in addition to the - /// `/.runtime/logs` buffer. On by default; disabled with - /// `SB_CHROME_LOG_CONSOLE=0`. pub log_console: bool, } impl ChromeConfig { - /// Build from the process environment. `server_url` is the loopback base - /// URL the headless page should load (`http://127.0.0.1:`); - /// `headless_token` is always generated by the caller; it's only consulted - /// by the auth layer when authentication is enabled. - pub fn from_env( - server_url: String, - headless_token: String, - space_folder: &str, - read_only: bool, - ) -> Option { + /// Build from the process environment. `server_root` is the server's root + /// directory, used for the default profile location. Returns `None` when + /// the runtime API is disabled or no Chrome can be found. + pub fn from_env(server_root: &Path) -> Option { let env = |k: &str| std::env::var(k).ok().filter(|v| !v.is_empty()); let runtime_api_enabled = !matches!(env("SB_RUNTIME_API").as_deref(), Some("0") | Some("false")); Self::resolve( env("SB_CHROME_PATH"), env("CHROMIUM_PATH"), - server_url, - headless_token, - space_folder.to_string(), + env("SB_CHROME_DATA_DIR"), + server_root, env("SB_CHROME_SHOW").is_some(), // On by default; disabled only with SB_CHROME_LOG_CONSOLE=0/false // (matches the SB_RUNTIME_API opt-out convention). @@ -45,42 +30,35 @@ impl ChromeConfig { env("SB_CHROME_LOG_CONSOLE").as_deref(), Some("0") | Some("false") ), - read_only, runtime_api_enabled, ) } - /// Pure resolution (unit-tested). Returns `None` when the runtime API is - /// disabled, the space is read-only, or no Chrome can be found. - #[allow(clippy::too_many_arguments)] + /// Pure resolution (unit-tested). Seven parameters is exactly clippy's + /// `too_many_arguments` threshold, so no `allow` is needed — do not add one. pub fn resolve( sb_chrome_path: Option, chromium_path: Option, - server_url: String, - headless_token: String, - space_folder: String, + chrome_data_dir: Option, + server_root: &Path, show: bool, log_console: bool, - read_only: bool, runtime_api_enabled: bool, ) -> Option { - if !runtime_api_enabled || read_only { + if !runtime_api_enabled { return None; } let chrome_path = sb_chrome_path.or(chromium_path).or_else(find_chrome)?; - let user_data_dir = std::env::var("SB_CHROME_DATA_DIR") - .ok() + let user_data_dir = chrome_data_dir .filter(|v| !v.is_empty()) .unwrap_or_else(|| { - Path::new(&space_folder) + server_root .join(".chrome-data") .to_string_lossy() .into_owned() }); Some(Self { chrome_path, - server_url, - headless_token, user_data_dir, show, log_console, @@ -88,6 +66,48 @@ impl ChromeConfig { } } +/// Per-space page configuration: where this space's client lives and how its +/// headless page authenticates to it. +#[derive(Debug, Clone)] +pub struct SpacePage { + /// Loopback base URL for this space (`http://127.0.0.1:`). + pub server_url: String, + /// Headless auth token, seeded as an HTTP-only session cookie before the + /// page navigates. Never appears in the URL. + pub headless_token: String, + /// This space's cookie name (`silverbullet_headless_`). + pub cookie_name: String, +} + +impl SpacePage { + /// The headless page URL: the space base URL with a trailing slash and + /// `?headless=1`. Authentication rides in the cookie, not the query string. + pub fn page_url(&self) -> String { + let base = self.server_url.trim_end_matches('/'); + format!("{base}/?headless=1") + } + + /// `Path` attribute for this space's auth cookie: the space prefix, or `/` + /// for a root-bound space. + pub fn cookie_path(&self) -> String { + let path = self + .server_url + .split_once("://") + .and_then(|(_, authority_and_path)| { + authority_and_path + .find('/') + .map(|index| &authority_and_path[index..]) + }) + .unwrap_or("/"); + let path = path.trim_end_matches('/'); + if path.is_empty() { + "/".to_string() + } else { + path.to_string() + } + } +} + /// Find a Chrome/Chromium executable from platform-specific candidates. pub fn find_chrome() -> Option { if cfg!(target_os = "macos") { @@ -167,100 +187,96 @@ fn which_on_path(name: &str) -> Option { mod tests { use super::*; - #[test] - fn explicit_path_wins_over_discovery() { - let cfg = ChromeConfig::resolve( - Some("/custom/chrome".into()), + fn resolve(root: &str, chrome_data_dir: Option<&str>) -> Option { + ChromeConfig::resolve( + Some("/bin/chrome".into()), None, - "http://127.0.0.1:3000".into(), - String::new(), - "/tmp/space".into(), - false, - false, + chrome_data_dir.map(str::to_string), + Path::new(root), false, true, - ); - assert_eq!(cfg.unwrap().chrome_path, "/custom/chrome"); - } - - #[test] - fn chromium_path_is_fallback_for_explicit() { - let cfg = ChromeConfig::resolve( - None, - Some("/usr/bin/chromium".into()), - "u".into(), - String::new(), - "/s".into(), - false, - false, - false, true, - ); - assert_eq!(cfg.unwrap().chrome_path, "/usr/bin/chromium"); - } - - #[test] - fn disabled_when_runtime_api_off_or_read_only() { - assert!(ChromeConfig::resolve( - Some("/x".into()), - None, - "u".into(), - String::new(), - "/s".into(), - false, - false, - false, - false ) - .is_none()); - assert!(ChromeConfig::resolve( - Some("/x".into()), + } + + #[test] + fn profile_defaults_to_the_server_root() { + let cfg = resolve("/srv/sb", None).expect("resolves"); + assert_eq!(cfg.chrome_path, "/bin/chrome"); + assert_eq!(cfg.user_data_dir, "/srv/sb/.chrome-data"); + } + + #[test] + fn explicit_data_dir_wins() { + let cfg = resolve("/srv/sb", Some("/var/cache/sb-chrome")).expect("resolves"); + assert_eq!(cfg.user_data_dir, "/var/cache/sb-chrome"); + } + + #[test] + fn empty_data_dir_is_ignored() { + let cfg = ChromeConfig::resolve( + Some("/bin/chrome".into()), None, - "u".into(), - String::new(), - "/s".into(), - false, + Some(String::new()), + Path::new("/srv/sb"), false, true, - true + true, + ) + .expect("resolves"); + assert_eq!(cfg.user_data_dir, "/srv/sb/.chrome-data"); + } + + #[test] + fn chromium_path_is_the_fallback() { + let cfg = ChromeConfig::resolve( + None, + Some("/bin/chromium".into()), + None, + Path::new("/srv/sb"), + false, + true, + true, + ) + .expect("resolves"); + assert_eq!(cfg.chrome_path, "/bin/chromium"); + } + + #[test] + fn disabled_runtime_api_resolves_to_nothing() { + assert!(ChromeConfig::resolve( + Some("/bin/chrome".into()), + None, + None, + Path::new("/srv/sb"), + false, + true, + false, ) .is_none()); } #[test] - fn data_dir_defaults_under_space() { - let cfg = ChromeConfig::resolve( - Some("/x".into()), - None, - "u".into(), - String::new(), - "/space".into(), - false, - false, - false, - true, - ) - .unwrap(); - assert!(cfg.user_data_dir.ends_with(".chrome-data")); - assert!(cfg.user_data_dir.contains("space")); + fn page_url_carries_no_token() { + let page = SpacePage { + server_url: "http://127.0.0.1:3000/notes".into(), + headless_token: "secret".into(), + cookie_name: "silverbullet_headless_a".into(), + }; + assert_eq!(page.page_url(), "http://127.0.0.1:3000/notes/?headless=1"); + assert!(!page.page_url().contains("secret")); } #[test] - fn token_show_and_log_console_flow_through() { - let cfg = ChromeConfig::resolve( - Some("/x".into()), - None, - "u".into(), - "tok123".into(), - "/s".into(), - true, - true, - false, - true, - ) - .unwrap(); - assert_eq!(cfg.headless_token, "tok123"); - assert!(cfg.show); - assert!(cfg.log_console); + fn cookie_path_is_the_space_prefix() { + let page = |url: &str| SpacePage { + server_url: url.into(), + headless_token: "secret".into(), + cookie_name: "silverbullet_headless_a".into(), + }; + assert_eq!(page("http://127.0.0.1:3000/notes").cookie_path(), "/notes"); + assert_eq!(page("http://127.0.0.1:3000").cookie_path(), "/"); + assert_eq!(page("http://127.0.0.1:3000/").cookie_path(), "/"); + assert_eq!(page("http://127.0.0.1:3000/a/b").cookie_path(), "/a/b"); } } diff --git a/server-runtime-chrome/src/lib.rs b/server-runtime-chrome/src/lib.rs index 9de2ed89..c1597d63 100644 --- a/server-runtime-chrome/src/lib.rs +++ b/server-runtime-chrome/src/lib.rs @@ -3,8 +3,8 @@ //! server can evaluate Space Lua and answer the objects API. mod config; +mod pool; mod supervisor; -mod transport; -pub use config::{find_chrome, ChromeConfig}; -pub use transport::ChromeTransport; +pub use config::{find_chrome, ChromeConfig, SpacePage}; +pub use pool::{ChromePool, SharedChromeTransport}; diff --git a/server-runtime-chrome/src/pool.rs b/server-runtime-chrome/src/pool.rs new file mode 100644 index 00000000..bf327cad --- /dev/null +++ b/server-runtime-chrome/src/pool.rs @@ -0,0 +1,478 @@ +//! The shared headless-Chrome pool: one browser process for the whole server, +//! one page per space. +//! +//! Launch is LAZY at two levels. The browser does not start until the first +//! runtime request from *any* space, and a space's page is not created until +//! that space's own first request — so Chrome never runs while the runtime API +//! is unused, and a space nobody queries never costs a tab. +//! +//! `eval_js`/`wait_ready` are synchronous and block the calling thread (the +//! server invokes them via `spawn_blocking`) they run their async work on the +//! pool's owned runtime. + +use std::collections::HashMap; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::{Arc, Mutex as StdMutex}; +use std::time::Duration; + +use chromiumoxide::browser::Browser; +use chromiumoxide::page::Page; +use serde_json::Value; +use silverbullet_server::runtime::{ClientTransport, LogBuffer, RuntimeError}; +use tokio::runtime::Runtime; +use tokio::sync::{Mutex, Notify}; + +use crate::config::{ChromeConfig, SpacePage}; +use crate::supervisor::{eval_on_page, launch_browser, supervise_space}; + +/// One registered space: its live page, readiness flag, and supervisor handle. +struct Registration { + live: Arc>>, + supervisor: tokio::task::JoinHandle<()>, +} + +/// The pool's browser slot: the live browser tagged with the generation it was +/// launched as, or empty when there is none. +type BrowserSlot = Arc)>>>; + +/// The shared headless-Chrome pool. +pub struct ChromePool { + rt: Option, + config: ChromeConfig, + browser: BrowserSlot, + launch_lock: Arc>, + spaces: Arc>>, + next_id: AtomicU64, + next_generation: AtomicU64, +} + +/// Clear `slot` when — and only when — it still holds `generation`. Returns +/// whether it cleared anything. +/// +/// This is the guard that stops a browser restart from ping-ponging: a +/// supervisor that noticed generation *G* had died may arrive long after +/// somebody else already launched *G+1* (a page launch takes seconds), and it +/// must not take the live browser down with it. +fn clear_if_generation(slot: &mut Option<(u64, T)>, generation: u64) -> bool { + match slot { + Some((current, _)) if *current == generation => { + *slot = None; + true + } + _ => false, + } +} + +impl ChromePool { + /// Construct the pool. Generic over the browser handle so tests can build a + /// pool whose slot they are able to populate; `ChromePool::new` is the + /// production entry point. + fn build(config: ChromeConfig) -> Result, RuntimeError> { + let rt = tokio::runtime::Builder::new_multi_thread() + .worker_threads(2) + .enable_all() + .build() + .map_err(|e| RuntimeError::Transport(format!("tokio runtime: {e}")))?; + Ok(Arc::new(Self { + rt: Some(rt), + config, + browser: Arc::new(Mutex::new(None)), + launch_lock: Arc::new(Mutex::new(())), + spaces: Arc::new(StdMutex::new(HashMap::new())), + next_id: AtomicU64::new(0), + next_generation: AtomicU64::new(0), + })) + } + + /// The owned runtime. Always present until `Drop`; the `Option` exists only + /// so `Drop` can take it. + fn rt(&self) -> &Runtime { + self.rt.as_ref().expect("runtime present until drop") + } + + pub fn config(&self) -> &ChromeConfig { + &self.config + } + + /// How many spaces currently hold a transport. Registration is per + /// *transport*, not per space id: a space being rebuilt briefly has two. + pub fn registered_spaces(&self) -> usize { + self.spaces.lock().unwrap_or_else(|e| e.into_inner()).len() + } + + /// Retire the browser of `generation` so the next `ensure_browser` launches + /// a fresh one. Called by a supervisor that saw *this* browser fail a + /// liveness probe. + pub(crate) async fn discard_browser(&self, generation: u64) { + // Taken before the slot lock so this cannot interleave with + // `ensure_browser`'s launch-then-store. + let _guard = self.launch_lock.lock().await; + let mut slot = self.browser.lock().await; + if clear_if_generation(&mut slot, generation) { + tracing::warn!("shared headless Chrome (generation {generation}) died; retiring it"); + } + } + + /// Whether a browser process is currently held. Exists to pin the laziness + /// guarantee in tests — nothing must launch Chrome before the first runtime + /// request. + #[cfg(test)] + pub(crate) async fn browser_is_launched(&self) -> bool { + self.browser.lock().await.is_some() + } + + /// Install `browser` in the slot at `generation`, as a successful + /// `ensure_browser` would. Tests only: it is the sole way to exercise + /// `discard_browser` without launching real Chrome. + #[cfg(test)] + async fn seed_browser(&self, generation: u64, browser: B) { + *self.browser.lock().await = Some((generation, Arc::new(browser))); + } +} + +impl ChromePool { + /// Create the pool. Returns an error only if the tokio runtime can't be + /// built. + pub fn new(config: ChromeConfig) -> Result, RuntimeError> { + Self::build(config) + } + + /// Register a space and return its transport. Spawns the space's supervisor + /// parked on its trigger; no browser or page work happens until the first + /// runtime request. + pub fn transport_for( + self: &Arc, + page: SpacePage, + logs: LogBuffer, + ) -> SharedChromeTransport { + let id = self.next_id.fetch_add(1, Ordering::Relaxed); + let live: Arc>> = Arc::new(Mutex::new(None)); + let ready = Arc::new(AtomicBool::new(false)); + let trigger = Arc::new(Notify::new()); + + let supervisor = self.rt().spawn(supervise_space( + self.clone(), + page.clone(), + live.clone(), + ready.clone(), + logs, + trigger.clone(), + )); + + self.spaces + .lock() + .unwrap_or_else(|e| e.into_inner()) + .insert( + id, + Registration { + live: live.clone(), + supervisor, + }, + ); + + SharedChromeTransport { + pool: self.clone(), + id, + live, + ready, + trigger, + } + } + + /// The live browser and its generation, launching it if there isn't one. + /// Concurrent callers serialize on `launch_lock`; the second one finds the + /// slot already filled and returns it. + /// + /// The returned `Arc` is a *transient* clone: use it for one `new_page` and + /// drop it. Holding it would keep a dead Chrome's process alive across a + /// relaunch. The generation goes back to `discard_browser` if this browser + /// turns out to be dead. + pub(crate) async fn ensure_browser(&self) -> Result<(u64, Arc), String> { + /// Cap on a single `Browser::launch`. It is awaited while holding + /// `launch_lock`, so a Chrome that hangs on startup would otherwise + /// block *every* space's supervisor forever. + const LAUNCH_TIMEOUT: Duration = Duration::from_secs(120); + + if let Some((generation, b)) = self.browser.lock().await.as_ref() { + return Ok((*generation, b.clone())); + } + let _guard = self.launch_lock.lock().await; + if let Some((generation, b)) = self.browser.lock().await.as_ref() { + return Ok((*generation, b.clone())); + } + tracing::info!( + "runtime API used; launching shared headless Chrome ({})", + self.config.chrome_path + ); + let browser = match tokio::time::timeout(LAUNCH_TIMEOUT, launch_browser(&self.config)).await + { + Err(_) => { + return Err(format!( + "browser launch timed out after {}s", + LAUNCH_TIMEOUT.as_secs() + )) + } + Ok(result) => Arc::new(result?), + }; + let generation = self.next_generation.fetch_add(1, Ordering::Relaxed); + *self.browser.lock().await = Some((generation, browser.clone())); + Ok((generation, browser)) + } +} + +impl Drop for ChromePool { + fn drop(&mut self) { + // Abort every supervisor first so none of them races the teardown. + for (_, reg) in self + .spaces + .lock() + .unwrap_or_else(|e| e.into_inner()) + .drain() + { + reg.supervisor.abort(); + drop(reg.live); + } + if let Some(rt) = self.rt.take() { + // `shutdown_background` rather than a blocking shutdown: the pool is + // dropped from inside the server's own async context on Ctrl-C, and + // blocking there panics. + rt.shutdown_background(); + } + } +} + +/// One space's view of the shared browser: its own page, readiness flag, and +/// lazy-launch trigger. +pub struct SharedChromeTransport { + pool: Arc, + id: u64, + live: Arc>>, + ready: Arc, + trigger: Arc, +} + +impl ClientTransport for SharedChromeTransport { + fn eval_js(&self, js: &str, timeout: Duration) -> Result { + // Any runtime use wakes this space's supervisor, which brings the shared + // browser up if needed and then this space's page. + self.trigger.notify_one(); + let live = self.live.clone(); + let js = js.to_string(); + self.pool.rt().block_on(async move { + // The timeout covers *acquiring* `live` as well as the eval itself. + // The eval alone was never unbounded — chromiumoxide gives every + // command a ~30s `REQUEST_TIMEOUT` of its own — but the lock wait in + // front of it had no bound at all, and it is the part that stacks: + // this call occupies a thread on the server's shared + // `spawn_blocking` pool, and the supervisor's liveness probe takes + // the same lock. Wrapping both means a caller's own timeout is the + // real bound, instead of an implicit one from a dependency plus an + // unbounded queue behind it. + let attempt = async { + let guard = live.lock().await; + let page = guard.as_ref().ok_or(RuntimeError::NotReady)?; + eval_on_page(page, &js).await + }; + match tokio::time::timeout(timeout, attempt).await { + Err(_) => Err(RuntimeError::Timeout), + Ok(r) => r, + } + }) + } + + fn wait_ready(&self, timeout: Duration) -> Result<(), RuntimeError> { + self.trigger.notify_one(); + if self.ready.load(Ordering::Relaxed) { + return Ok(()); + } + let ready = self.ready.clone(); + self.pool.rt().block_on(async move { + let deadline = tokio::time::Instant::now() + timeout; + loop { + if ready.load(Ordering::Relaxed) { + return Ok(()); + } + if tokio::time::Instant::now() >= deadline { + return Err(RuntimeError::NotReady); + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + }) + } + + fn is_ready(&self) -> bool { + self.ready.load(Ordering::Relaxed) + } + + fn ensure_started(&self) { + // Same lazy-launch nudge as eval/wait: a log read alone is enough to + // bring this space's page up. + self.trigger.notify_one(); + } +} + +impl Drop for SharedChromeTransport { + fn drop(&mut self) { + // A space instance is rebuilt on *every* admin API space write, so + // without this each write would leak a tab in the shared browser. + let reg = self + .pool + .spaces + .lock() + .unwrap_or_else(|e| e.into_inner()) + .remove(&self.id); + let Some(reg) = reg else { return }; + let Registration { live, supervisor } = reg; + self.pool.rt().spawn(async move { + // Abort *and wait* before touching `live`. `JoinHandle::abort` is + // not synchronous: cancelling the supervisor while it is inside + // `launch_page` means its page is a task local, in neither `live` + // nor anywhere else, until its `PageCloseGuard` drops. Aborting from + // `Drop` and closing here immediately could observe `live == None`, + // finish, and let the supervisor then publish a page nobody will + // ever look at again. Awaiting the handle guarantees the task (and + // its guard) is fully torn down first. + supervisor.abort(); + let _ = supervisor.await; + // Two statements, so the `MutexGuard` is dropped at the `;` rather + // than held across the `close` await — matching `supervise_space`'s + // restart path. Nothing contends for `live` here, but the two sites + // should read alike. + let stale = live.lock().await.take(); + if let Some(page) = stale { + let _ = page.close().await; + } + }); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn config() -> ChromeConfig { + ChromeConfig { + // Never launched: registration and teardown are lazy, so no real + // browser is needed to exercise the pool's bookkeeping. + chrome_path: "/nonexistent/chrome".into(), + user_data_dir: "/tmp/sb-pool-test".into(), + show: false, + log_console: false, + } + } + + fn page(name: &str, url: &str) -> SpacePage { + SpacePage { + server_url: url.into(), + headless_token: "secret".into(), + cookie_name: format!("silverbullet_headless_{name}"), + } + } + + #[test] + fn registers_and_deregisters_each_space() { + let pool = ChromePool::new(config()).unwrap(); + assert_eq!(pool.registered_spaces(), 0); + + let a = pool.transport_for(page("a", "http://127.0.0.1:3000"), LogBuffer::new()); + let b = pool.transport_for(page("b", "http://127.0.0.1:3000/notes"), LogBuffer::new()); + assert_eq!(pool.registered_spaces(), 2); + + drop(a); + assert_eq!(pool.registered_spaces(), 1); + drop(b); + assert_eq!(pool.registered_spaces(), 0); + } + + #[test] + fn rebuilding_a_space_does_not_deregister_its_replacement() { + let pool = ChromePool::new(config()).unwrap(); + let old = pool.transport_for(page("a", "http://127.0.0.1:3000"), LogBuffer::new()); + let new = pool.transport_for(page("a", "http://127.0.0.1:3000"), LogBuffer::new()); + assert_eq!(pool.registered_spaces(), 2); + + drop(old); + assert_eq!(pool.registered_spaces(), 1); + drop(new); + assert_eq!(pool.registered_spaces(), 0); + } + + #[tokio::test] + async fn no_browser_is_launched_before_any_runtime_request() { + let pool = ChromePool::new(config()).unwrap(); + assert!(!pool.browser_is_launched().await); + + let t = pool.transport_for(page("a", "http://127.0.0.1:3000"), LogBuffer::new()); + // Give the supervisor task a chance to run before checking. + tokio::time::sleep(Duration::from_millis(50)).await; + assert!(!pool.browser_is_launched().await); + assert!(!t.is_ready()); + } + + #[test] + fn a_stale_discard_leaves_a_newer_browser_alone() { + let mut slot = Some((1u64, "browser-1")); + assert!(!clear_if_generation(&mut slot, 0)); + assert_eq!(slot, Some((1, "browser-1"))); + } + + #[test] + fn discarding_the_current_generation_clears_the_slot() { + let mut slot = Some((1u64, "browser-1")); + assert!(clear_if_generation(&mut slot, 1)); + assert_eq!(slot, None); + } + + #[tokio::test] + async fn discard_browser_retires_only_the_generation_it_was_given() { + let pool = ChromePool::<&'static str>::build(config()).unwrap(); + pool.seed_browser(1, "browser-1").await; + + // A supervisor that watched generation 0 die, arriving after somebody + // else already launched generation 1, must leave it alone. + pool.discard_browser(0).await; + assert!( + pool.browser_is_launched().await, + "a stale generation must not retire the live browser" + ); + + // The generation that actually failed is retired, so the next + // `ensure_browser` launches a fresh one. + pool.discard_browser(1).await; + assert!( + !pool.browser_is_launched().await, + "the current generation must clear the slot" + ); + } + + #[test] + fn discarding_an_empty_slot_is_a_no_op() { + let mut slot: Option<(u64, &str)> = None; + assert!(!clear_if_generation(&mut slot, 0)); + assert_eq!(slot, None); + } + + #[test] + fn generations_are_monotonic() { + let pool = ChromePool::new(config()).unwrap(); + let first = pool.next_generation.fetch_add(1, Ordering::Relaxed); + let second = pool.next_generation.fetch_add(1, Ordering::Relaxed); + assert_eq!((first, second), (0, 1)); + } + + #[tokio::test] + async fn a_failed_launch_leaves_the_slot_empty() { + let pool = ChromePool::new(config()).unwrap(); + // `/nonexistent/chrome` cannot be spawned, so this fails fast. + assert!(pool.ensure_browser().await.is_err()); + assert!(!pool.browser_is_launched().await); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn dropping_the_pool_inside_an_async_context_does_not_panic() { + let pool = ChromePool::new(config()).unwrap(); + let t = pool.transport_for(page("a", "http://127.0.0.1:3000"), LogBuffer::new()); + drop(t); + drop(pool); // must not panic + } +} diff --git a/server-runtime-chrome/src/supervisor.rs b/server-runtime-chrome/src/supervisor.rs index bff1785c..82e9fca6 100644 --- a/server-runtime-chrome/src/supervisor.rs +++ b/server-runtime-chrome/src/supervisor.rs @@ -1,17 +1,19 @@ -//! Browser supervisor: owns the live `Browser` + `Page`, captures console -//! output into the shared log buffer, waits for the client runtime to signal +//! Browser launch and per-space page supervision: brings up the shared +//! `Browser` for the pool, opens a space's `Page` in it, captures console output +//! into that space's log buffer, waits for the client runtime to signal //! readiness, and restarts (with exponential backoff) whenever the page dies. //! -//! The supervisor runs as a single long-lived task on the transport's tokio -//! runtime. It is deliberately tolerant of a not-yet-listening server: the -//! transport is constructed *before* the server binds its port, so the first -//! navigation may fail and is simply retried with backoff. +//! One supervisor task per space runs on the pool's tokio runtime. It is +//! deliberately tolerant of a not-yet-listening server: transports are +//! constructed *before* the server binds its port, so the first navigation may +//! fail and is simply retried with backoff. use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use chromiumoxide::browser::{Browser, BrowserConfig}; +use chromiumoxide::cdp::browser_protocol::network::{CookieParam, CookieSameSite}; use chromiumoxide::cdp::js_protocol::runtime::{ ConsoleApiCalledType, EvaluateParams, EventConsoleApiCalled, }; @@ -22,17 +24,7 @@ use serde_json::Value; use silverbullet_server::runtime::{LogBuffer, LogEntry, RuntimeError}; use tokio::sync::{Mutex, Notify}; -use crate::config::ChromeConfig; - -/// The live browser and its single page, kept together so dropping the pair -/// (on restart) tears down the old Chrome process cleanly. -pub struct Live { - /// Held purely to keep the Chrome process alive; dropping `Live` (on - /// restart) drops this and tears Chrome down. Never read directly. - #[allow(dead_code)] - pub browser: Browser, - pub page: Page, -} +use crate::config::{ChromeConfig, SpacePage}; /// Evaluate a raw JS expression in the page with await-promise + /// return-by-value semantics, returning its JSON value (`Null` when the @@ -103,6 +95,18 @@ async fn eval_sync(page: &Page, js: &str) -> Result { Ok(result.value().cloned().unwrap_or(Value::Null)) } +/// Bound on the CDP round trips this module makes against a page it has reason +/// to distrust: the liveness probe, and every `close`. +/// +/// Nothing here was ever *unbounded* — chromiumoxide gives every command its own +/// `REQUEST_TIMEOUT` (~30s) and resolves it to `CdpError::Timeout` — but 30s is +/// the wrong bound for these call sites, and inheriting it from a dependency's +/// internals leaves the policy invisible. A liveness probe evaluates the literal +/// `1` and a close is a single command; five seconds is already enormously +/// generous for either, and the difference between 5s and 30s is paid by the +/// server (see `page_is_alive`) or by a space's restart latency. +const PROBE_TIMEOUT: Duration = Duration::from_secs(5); + /// Current wall-clock time in milliseconds since the Unix epoch. fn now_millis() -> i64 { SystemTime::now() @@ -111,30 +115,20 @@ fn now_millis() -> i64 { .unwrap_or(0) } -/// Build the headless page URL from the configured server URL, trimming a -/// trailing slash and appending `?headless=1&token=…`. The token is always -/// present; an open (no-auth) server simply ignores it. -fn page_url(config: &ChromeConfig) -> String { - let base = config.server_url.trim_end_matches('/'); - format!("{base}/?headless=1&token={}", config.headless_token) +/// The space's headless auth cookie: HTTP-only, `SameSite=Strict`, scoped by +/// `Path` to the space prefix, with no expiry (a session cookie). +pub(crate) fn headless_cookie(page: &SpacePage) -> Result { + CookieParam::builder() + .name(&page.cookie_name) + .value(&page.headless_token) + .url(&page.server_url) + .path(page.cookie_path()) + .http_only(true) + .same_site(CookieSameSite::Strict) + .build() } -/// Launch a fresh browser + page, attach console capture, wait for the client -/// runtime to become ready, then publish the live pair into `live_slot` and -/// set `ready = true`. Returns `Err(msg)` on any failure (the caller backs off -/// and retries). -async fn launch_once( - config: &ChromeConfig, - live_slot: &Arc>>, - ready: &Arc, - logs: &LogBuffer, -) -> Result<(), String> { - // chromiumoxide's `.args()` expects BARE flag keys (no leading `--`) — it - // prepends the dashes itself when building the command line, so a value like - // `"--no-sandbox"` is mangled to `----no-sandbox` and silently ignored. Use - // the dedicated `.no_sandbox()` method (it emits the correct - // `--no-sandbox` + `--disable-setuid-sandbox`, required when running as root, - // e.g. inside a container) and pass the remaining flags as bare keys. +pub(crate) async fn launch_browser(config: &ChromeConfig) -> Result { let mut builder = BrowserConfig::builder() .chrome_executable(&config.chrome_path) .user_data_dir(&config.user_data_dir) @@ -164,18 +158,127 @@ async fn launch_once( } }); - let page = browser - .new_page(page_url(config).as_str()) - .await - .map_err(|e| format!("new page: {e}"))?; + Ok(browser) +} - attach_console_capture(&page, logs, config.log_console) +/// A page handle whose tab must be closed explicitly rather than dropped. +/// +/// Abstracted behind a trait for exactly one reason: `PageCloseGuard`'s +/// behaviour under task cancellation is the thing this module most needs to +/// test, and `chromiumoxide::Page` cannot be constructed without a live Chrome. +/// Production only ever instantiates it at `P = Page`. +/// +/// Declared with an explicit `-> impl Future + Send` rather than `async fn`: the +/// close is handed to `tokio::spawn`, which needs the `Send` bound stated on the +/// trait itself. +trait ClosablePage: Send + 'static { + fn close_page(self) -> impl std::future::Future + Send; +} + +impl ClosablePage for Page { + async fn close_page(self) { + // Explicitly bounded, rather than leaning on chromiumoxide's ~30s + // request timeout: this frequently runs on a page already declared dead. + let _ = tokio::time::timeout(PROBE_TIMEOUT, self.close()).await; + } +} + +/// Owns a freshly opened page and **closes** it on drop, unless disarmed. +/// +/// chromiumoxide implements `Drop` for `Browser` but *not* for `Page` — +/// `Page::close` is an explicit `async fn` — so a dropped `Page` leaves its tab +/// and renderer process running inside the browser every other space depends +/// on. This used to be impossible: each attempt owned its own `Browser`, so a +/// failed attempt took the whole process tree with it. Sharing the browser +/// removed that implicit cleanup, and this guard replaces it. +struct PageCloseGuard { + page: Option

, +} + +impl PageCloseGuard

{ + fn new(page: P) -> Self { + Self { page: Some(page) } + } + + /// The guarded page. Borrowed, never moved out: ownership stays with the + /// guard so cancellation at *any* await point still reaches `Drop`. + fn page(&self) -> &P { + self.page.as_ref().expect("armed until disarmed") + } + + /// Give up responsibility for the page. Only correct once the page is + /// published somewhere that will close it — i.e. `live`. + fn disarm(&mut self) { + self.page = None; + } +} + +impl Drop for PageCloseGuard

{ + fn drop(&mut self) { + if let Some(page) = self.page.take() { + // Fire-and-forget: `Drop` cannot await. See the type-level comment + // for why a runtime is guaranteed to be present here. + tokio::spawn(page.close_page()); + } + } +} + +/// Open this space's page in the shared browser: blank page, auth cookie, +/// console capture, navigation, then wait for the client runtime to report +/// ready. Publishes into `live` and sets `ready` on success. +/// +/// The tab is owned by a `PageCloseGuard` for the whole of this function, so +/// every exit — `?`, a returned `Err`, or the task being aborted at any of the +/// await points — closes it. That is the single mechanism; there is +/// deliberately no separate close on the error paths. Without it, a space that +/// keeps timing out in `wait_for_client_ready` (a large space still indexing on +/// first boot is the likeliest trigger), or one whose supervisor is aborted +/// mid-launch by an admin API space write, abandons a live renderer per attempt, +/// forever — the browser never looks dead, so nothing ever retires it. +/// +/// One residual window is out of reach: an abort landing *inside* `new_page` +/// itself can leave a target that was created browser-side but whose handle +/// never reached this task. It is a single CDP round trip rather than a 60s +/// poll loop, so the exposure is tiny compared to what the guard covers. +async fn launch_page( + browser: &Browser, + page_cfg: &SpacePage, + log_console: bool, + live: &Arc>>, + ready: &Arc, + logs: &LogBuffer, +) -> Result<(), String> { + let mut guard = PageCloseGuard::new( + browser + .new_page("about:blank") + .await + .map_err(|e| format!("new page: {e}"))?, + ); + let page = guard.page(); + + page.set_cookie(headless_cookie(page_cfg)?) + .await + .map_err(|e| format!("headless auth cookie: {e}"))?; + + // Console capture is attached *before* navigating: attaching afterwards + // deterministically misses the client's earliest boot output, which is + // exactly what an operator needs to debug a space that never reaches ready. + attach_console_capture(page, logs, log_console) .await .map_err(|e| format!("console capture: {e}"))?; - wait_for_client_ready(&page).await?; + page.goto(page_cfg.page_url().as_str()) + .await + .map_err(|e| format!("headless page navigation: {e}"))?; - *live_slot.lock().await = Some(Live { browser, page }); + wait_for_client_ready(page).await?; + + // Publish first, disarm second, and never await in between: from here on the + // supervisor (or `SharedChromeTransport::Drop`) closes the page via `live`. + // `Page` is `Clone` and the clones share one underlying tab, so closing any + // of them closes it. + *live.lock().await = Some(page.clone()); + guard.disarm(); ready.store(true, Ordering::Relaxed); Ok(()) } @@ -267,29 +370,30 @@ async fn wait_for_client_ready(page: &Page) -> Result<(), String> { } } -/// Is the current page still alive? A trivial eval that errors means the page -/// (or the whole browser) is gone. -async fn page_is_alive(live_slot: &Arc>>) -> bool { - let guard = live_slot.lock().await; +/// Is this space's page still alive? A trivial eval that errors — or that never +/// answers — means the page (or the whole browser) is gone. +async fn page_is_alive(live: &Arc>>) -> bool { + let guard = live.lock().await; match guard.as_ref() { None => false, - Some(live) => eval_sync(&live.page, "1").await.is_ok(), + // A timed-out probe reports *not* alive, so the supervisor takes the + // restart path rather than waiting on a page that will never answer. + Some(page) => matches!( + tokio::time::timeout(PROBE_TIMEOUT, eval_sync(page, "1")).await, + Ok(Ok(_)) + ), } } -/// Supervise the browser for the lifetime of the transport. -/// -/// Loop shape: each iteration first checks liveness. When the page is dead -/// (the initial `None` slot, or a crashed page), we mark the runtime not-ready, -/// drop any stale `Live`, and attempt `launch_once`. On success we reset the -/// backoff to its floor and continue; on failure we sleep for the current -/// backoff (2s, doubling up to 120s) before the next attempt. When the page is -/// alive we sleep ~2s between liveness checks. The browser is launched LAZILY: -/// the supervisor idles until the first runtime request (`requested`) before its -/// first `launch_once`, so Chrome never starts while the runtime API is unused. -pub async fn supervise( - config: ChromeConfig, - live_slot: Arc>>, +async fn browser_is_dead(browser: &Browser) -> bool { + browser.pages().await.is_err() +} + +/// Supervise one space's page for the lifetime of its transport. +pub(crate) async fn supervise_space( + pool: Arc, + page_cfg: SpacePage, + live: Arc>>, ready: Arc, logs: LogBuffer, trigger: Arc, @@ -298,41 +402,65 @@ pub async fn supervise( const BACKOFF_CAP: Duration = Duration::from_secs(120); const LIVENESS_INTERVAL: Duration = Duration::from_secs(2); - // Lazy launch: park here until the first runtime request, so Chrome is not - // started (and emits no CDP noise) while the runtime API is unused. `Notify` - // suspends the task with no polling; if the request arrived first, - // `notify_one` already stored a permit so this returns immediately. trigger.notified().await; - tracing::info!( - "runtime API used; launching headless Chrome ({})", - config.chrome_path - ); let mut backoff = BACKOFF_FLOOR; let mut has_launched = false; loop { - if page_is_alive(&live_slot).await { + if page_is_alive(&live).await { tokio::time::sleep(LIVENESS_INTERVAL).await; continue; } - // The page is dead (or never launched): mark not-ready and drop any - // stale browser before relaunching. A dead page *after* a successful - // launch is a crash/restart, worth flagging distinctly from first boot. if has_launched { - tracing::warn!("headless Chrome page died; restarting"); + tracing::warn!("headless page for {} died; restarting", page_cfg.server_url); } ready.store(false, Ordering::Relaxed); - *live_slot.lock().await = None; + let stale = live.lock().await.take(); + if let Some(old) = stale { + let _ = tokio::time::timeout(PROBE_TIMEOUT, old.close()).await; + } - match launch_once(&config, &live_slot, &ready, &logs).await { + // `browser` is deliberately scoped to this block: the pool holds the + // owning handle, and a lingering clone here would keep a dead Chrome + // process alive across a relaunch. + let attempt = match pool.ensure_browser().await { + Ok((generation, browser)) => { + let result = launch_page( + &browser, + &page_cfg, + pool.config().log_console, + &live, + &ready, + &logs, + ) + .await; + // Retire the browser only when it is genuinely gone. Discarding + // on every failed attempt would let one space's persistent + // navigation failure keep killing the browser for everyone. + let dead = result.is_err() && browser_is_dead(&browser).await; + // Drop the transient clone *before* retiring the pool's handle, + // so clearing the slot really does release the process tree. + drop(browser); + if dead { + pool.discard_browser(generation).await; + } + result + } + Err(e) => Err(e), + }; + + match attempt { Ok(()) => { has_launched = true; backoff = BACKOFF_FLOOR; - tracing::info!("headless Chrome runtime ready"); + tracing::info!("headless runtime ready for {}", page_cfg.server_url); } Err(e) => { - tracing::warn!("headless chrome launch failed: {e}; retrying in {backoff:?}"); + tracing::warn!( + "headless page for {} failed to start: {e}; retrying in {backoff:?}", + page_cfg.server_url + ); tokio::time::sleep(backoff).await; backoff = (backoff * 2).min(BACKOFF_CAP); } @@ -342,8 +470,190 @@ pub async fn supervise( #[cfg(test)] mod tests { + use std::sync::atomic::AtomicUsize; + use super::*; + #[derive(Clone)] + struct FakePage { + closed: Arc, + } + + impl ClosablePage for FakePage { + async fn close_page(self) { + self.closed.fetch_add(1, Ordering::SeqCst); + } + } + + async fn launch_page_shaped( + page: FakePage, + live: Arc>>, + ready: Arc, + entered: Arc, + hold: Arc, + ) { + let mut guard = PageCloseGuard::new(page); + let page = guard.page(); + entered.notify_one(); + + // set_cookie / attach_console_capture / goto. + for _ in 0..3 { + tokio::task::yield_now().await; + } + // wait_for_client_ready. + hold.notified().await; + + *live.lock().await = Some(page.clone()); + guard.disarm(); + ready.store(true, Ordering::Relaxed); + } + + /// Wait (briefly) for the detached close task spawned by `Drop` to run. + async fn await_closes(closed: &Arc, want: usize) -> usize { + for _ in 0..200 { + let n = closed.load(Ordering::SeqCst); + if n >= want { + return n; + } + tokio::time::sleep(Duration::from_millis(5)).await; + } + closed.load(Ordering::SeqCst) + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn aborting_a_launch_mid_flight_closes_the_page() { + let closed = Arc::new(AtomicUsize::new(0)); + let live = Arc::new(Mutex::new(None)); + let ready = Arc::new(AtomicBool::new(false)); + let entered = Arc::new(Notify::new()); + let hold = Arc::new(Notify::new()); + + let task = tokio::spawn(launch_page_shaped( + FakePage { + closed: closed.clone(), + }, + live.clone(), + ready.clone(), + entered.clone(), + hold.clone(), + )); + + // Let it reach the readiness wait, then cancel it there. + entered.notified().await; + tokio::time::sleep(Duration::from_millis(50)).await; + assert_eq!(closed.load(Ordering::SeqCst), 0, "nothing closed yet"); + task.abort(); + let _ = task.await; + + assert_eq!( + await_closes(&closed, 1).await, + 1, + "a cancelled launch must close its page, not drop the handle" + ); + assert!(live.lock().await.is_none()); + assert!(!ready.load(Ordering::Relaxed)); + } + + /// Cancellation before the readiness wait — the earlier await points + /// (`set_cookie`, console capture, `goto`) — is covered too. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn aborting_a_launch_during_setup_closes_the_page() { + let closed = Arc::new(AtomicUsize::new(0)); + let live = Arc::new(Mutex::new(None)); + let entered = Arc::new(Notify::new()); + let hold = Arc::new(Notify::new()); + + let task = tokio::spawn(launch_page_shaped( + FakePage { + closed: closed.clone(), + }, + live.clone(), + Arc::new(AtomicBool::new(false)), + entered.clone(), + hold, + )); + // Abort as soon as the guard exists — no sleep — so the cancellation + // lands in the setup awaits rather than the readiness wait. + entered.notified().await; + task.abort(); + let _ = task.await; + + assert_eq!(await_closes(&closed, 1).await, 1); + assert!(live.lock().await.is_none()); + } + + /// The other half: a launch that *succeeds* must not close the page it just + /// published. Ownership moves to `live`, and the supervisor closes it from + /// there on restart. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn a_successful_launch_does_not_close_the_page() { + let closed = Arc::new(AtomicUsize::new(0)); + let live = Arc::new(Mutex::new(None)); + let ready = Arc::new(AtomicBool::new(false)); + let entered = Arc::new(Notify::new()); + let hold = Arc::new(Notify::new()); + + let task = tokio::spawn(launch_page_shaped( + FakePage { + closed: closed.clone(), + }, + live.clone(), + ready.clone(), + entered, + hold.clone(), + )); + hold.notify_one(); + task.await.unwrap(); + + // Long enough that a stray close task would have landed by now. + tokio::time::sleep(Duration::from_millis(50)).await; + assert_eq!(closed.load(Ordering::SeqCst), 0); + assert!(live.lock().await.is_some()); + assert!(ready.load(Ordering::Relaxed)); + } + + /// An `Err` return closes the tab through the same single mechanism — there + /// is deliberately no separate close on `launch_page`'s error paths. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn an_error_return_closes_the_page_via_the_guard() { + let closed = Arc::new(AtomicUsize::new(0)); + let c = closed.clone(); + let task = tokio::spawn(async move { + let guard = PageCloseGuard::new(FakePage { closed: c }); + let _ = guard.page(); + Err::<(), String>("headless page navigation: boom".to_string()) + }); + assert!(task.await.unwrap().is_err()); + assert_eq!(await_closes(&closed, 1).await, 1); + } + + fn page_cfg(server_url: &str) -> SpacePage { + SpacePage { + server_url: server_url.to_string(), + headless_token: "secret".to_string(), + cookie_name: "silverbullet_headless_a".to_string(), + } + } + + #[test] + fn headless_cookie_is_http_only_and_named_per_space() { + let cookie = headless_cookie(&page_cfg("http://127.0.0.1:3000")).unwrap(); + assert_eq!(cookie.name, "silverbullet_headless_a"); + assert_eq!(cookie.value, "secret"); + assert_eq!(cookie.url.as_deref(), Some("http://127.0.0.1:3000")); + assert_eq!(cookie.path.as_deref(), Some("/")); + assert_eq!(cookie.http_only, Some(true)); + assert_eq!(cookie.same_site, Some(CookieSameSite::Strict)); + // Session cookie: it must not outlive the browser. + assert_eq!(cookie.expires, None); + } + + #[test] + fn headless_cookie_is_scoped_to_the_space_prefix() { + let cookie = headless_cookie(&page_cfg("http://127.0.0.1:3000/notes")).unwrap(); + assert_eq!(cookie.path.as_deref(), Some("/notes")); + } + #[test] fn console_level_maps_severity() { assert_eq!( diff --git a/server-runtime-chrome/src/transport.rs b/server-runtime-chrome/src/transport.rs deleted file mode 100644 index 406a63e8..00000000 --- a/server-runtime-chrome/src/transport.rs +++ /dev/null @@ -1,193 +0,0 @@ -//! The headless-Chrome `ClientTransport`: owns a dedicated tokio runtime and a -//! supervised browser page running the normal client in `?headless=1` mode. -//! -//! `eval_js`/`wait_ready` are synchronous and block the calling thread (the -//! server invokes them via `spawn_blocking`); they run their async work on the -//! transport's owned runtime. Launch is LAZY and tolerant: the supervisor does -//! not start Chrome until the first runtime request (`eval_js`/`wait_ready`), -//! then brings the browser up and keeps it alive — so the server can finish -//! binding its port before the headless page connects, and Chrome never runs -//! while the runtime API is unused. Until the page reports `sbRuntime.ready`, -//! `is_ready()` is false and eval/wait return `NotReady`/`Timeout` (→ the -//! server answers 503/504). - -use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; -use std::time::Duration; - -use serde_json::Value; -use silverbullet_server::runtime::{ClientTransport, LogBuffer, RuntimeError}; -use tokio::runtime::Runtime; -use tokio::sync::{Mutex, Notify}; - -use crate::config::ChromeConfig; -use crate::supervisor::{eval_on_page, Live}; - -pub struct ChromeTransport { - rt: Option, - live: Arc>>, - ready: Arc, - /// Notified on the first runtime request; the supervisor parks on this - /// before launching Chrome, so the browser only starts when the runtime API - /// is actually used. - trigger: Arc, - _supervisor: tokio::task::JoinHandle<()>, -} - -impl ChromeTransport { - /// Launch Chrome and start the supervisor. Pushes console output into `logs` - /// (pass a clone of the `LogBuffer` you give to `ClientRuntime`). Returns an - /// error only if the tokio runtime can't be created — the browser launch and - /// page readiness are handled (and retried) by the supervisor, so the server - /// can finish binding its port before the headless page connects. - pub fn launch(config: ChromeConfig, logs: LogBuffer) -> Result { - let rt = tokio::runtime::Builder::new_multi_thread() - .worker_threads(2) - .enable_all() - .build() - .map_err(|e| RuntimeError::Transport(format!("tokio runtime: {e}")))?; - let live = Arc::new(Mutex::new(None)); - let ready = Arc::new(AtomicBool::new(false)); - let trigger = Arc::new(Notify::new()); - let supervisor = { - let (live, ready, logs, config, trigger) = ( - live.clone(), - ready.clone(), - logs.clone(), - config.clone(), - trigger.clone(), - ); - rt.spawn(crate::supervisor::supervise( - config, live, ready, logs, trigger, - )) - }; - Ok(Self { - rt: Some(rt), - live, - ready, - trigger, - _supervisor: supervisor, - }) - } - - /// The owned runtime. Always present until `Drop`; the `Option` exists only - /// so `Drop` can take it. - fn rt(&self) -> &Runtime { - self.rt.as_ref().expect("runtime present until drop") - } -} - -impl Drop for ChromeTransport { - fn drop(&mut self) { - if let Some(rt) = self.rt.take() { - rt.shutdown_background(); - } - } -} - -impl ClientTransport for ChromeTransport { - fn eval_js(&self, js: &str, timeout: Duration) -> Result { - // Any runtime use wakes the supervisor to launch Chrome. - self.trigger.notify_one(); - let live = self.live.clone(); - let js = js.to_string(); - self.rt().block_on(async move { - let guard = live.lock().await; - let live = guard.as_ref().ok_or(RuntimeError::NotReady)?; - match tokio::time::timeout(timeout, eval_on_page(&live.page, &js)).await { - Err(_) => Err(RuntimeError::Timeout), - Ok(r) => r, - } - }) - } - - fn wait_ready(&self, timeout: Duration) -> Result<(), RuntimeError> { - // First runtime request wakes the supervisor to launch Chrome. - self.trigger.notify_one(); - if self.ready.load(Ordering::Relaxed) { - return Ok(()); - } - let ready = self.ready.clone(); - self.rt().block_on(async move { - let deadline = tokio::time::Instant::now() + timeout; - loop { - if ready.load(Ordering::Relaxed) { - return Ok(()); - } - if tokio::time::Instant::now() >= deadline { - return Err(RuntimeError::NotReady); - } - tokio::time::sleep(Duration::from_millis(100)).await; - } - }) - } - - fn is_ready(&self) -> bool { - self.ready.load(Ordering::Relaxed) - } - - fn ensure_started(&self) { - // Same lazy-launch nudge as eval/wait: wake the supervisor so a - // log read alone is enough to bring Chrome up. - self.trigger.notify_one(); - } -} - -#[cfg(test)] -mod tests { - use super::*; - use silverbullet_server::runtime::LogBuffer; - - /// Regression test for the Ctrl-C shutdown panic: the transport owns a - /// multi-thread runtime and is dropped while the outer server runtime is - /// still active (here, the multi-thread `#[tokio::test]` runtime, matching - /// `#[tokio::main]`). - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn dropping_transport_inside_async_context_does_not_panic() { - let cfg = crate::config::ChromeConfig::resolve( - Some("/nonexistent/chrome".into()), - None, - "http://127.0.0.1:0".into(), - String::new(), - "/tmp".into(), - false, - false, - false, - true, - ) - .expect("config resolves with an explicit chrome path"); - let transport = ChromeTransport::launch(cfg, LogBuffer::new()).unwrap(); - drop(transport); // must not panic - } - - #[test] - fn eval_against_live_server_when_configured() { - let Ok(url) = std::env::var("SB_TEST_RUNTIME_URL") else { - eprintln!("skip: set SB_TEST_RUNTIME_URL to run"); - return; - }; - let Some(cfg) = crate::config::ChromeConfig::resolve( - None, - None, - url, - String::new(), - "/tmp/x".into(), - false, - false, - false, - true, - ) else { - eprintln!("skip: no Chrome"); - return; - }; - let t = ChromeTransport::launch(cfg, LogBuffer::new()).unwrap(); - t.wait_ready(std::time::Duration::from_secs(60)).unwrap(); - let v = t - .eval_js( - "sbRuntime.evalLua(\"return 1 + 1\")", - std::time::Duration::from_secs(10), - ) - .unwrap(); - assert_eq!(v, serde_json::json!(2)); - } -} diff --git a/server/src/auth/headless_token.rs b/server/src/auth/headless_token.rs index 6f954c22..711756fa 100644 --- a/server/src/auth/headless_token.rs +++ b/server/src/auth/headless_token.rs @@ -1,38 +1,51 @@ +use axum::http::HeaderMap; + use crate::auth::authorizer::{AuthContext, RequestAuthorizer}; use crate::auth::config::constant_time_eq; +use crate::auth::cookie::cookie_value; + +/// Session-cookie name carrying a space's headless token. +/// +/// Every space gets its own name. The server-managed browser is shared by all +/// spaces, so all their cookies land in one jar; `Path` scoping alone is +/// ambiguous because a space bound at the root prefix sets `Path=/` and its +/// cookie then rides along on every other space's requests. A per-space name +/// removes any dependence on the browser's cookie ordering. +pub fn headless_cookie_name(space_id: &str) -> String { + format!("silverbullet_headless_{space_id}") +} /// Wraps an inner authorizer, additionally accepting any request that carries -/// `?token=` in its query string (constant-time compared). This -/// lets the headless browser page authenticate via the URL the server hands it. +/// this space's headless token in its dedicated session cookie. pub struct HeadlessTokenAuthorizer { inner: Box, + cookie_name: String, token: String, } impl HeadlessTokenAuthorizer { - pub fn new(inner: Box, token: String) -> Self { - Self { inner, token } + pub fn new(inner: Box, cookie_name: String, token: String) -> Self { + Self { + inner, + cookie_name, + token, + } } - fn query_token_matches(&self, query: Option<&str>) -> bool { + fn cookie_token_matches(&self, headers: &HeaderMap) -> bool { if self.token.is_empty() { return false; } - let Some(q) = query else { return false }; - for pair in q.split('&') { - if let Some(v) = pair.strip_prefix("token=") { - if constant_time_eq(v.as_bytes(), self.token.as_bytes()) { - return true; - } - } + match cookie_value(headers, &self.cookie_name) { + Some(v) => constant_time_eq(v.as_bytes(), self.token.as_bytes()), + None => false, } - false } } impl RequestAuthorizer for HeadlessTokenAuthorizer { fn is_authorized(&self, ctx: &AuthContext) -> bool { - self.query_token_matches(ctx.query) || self.inner.is_authorized(ctx) + self.cookie_token_matches(ctx.headers) || self.inner.is_authorized(ctx) } } @@ -63,35 +76,107 @@ mod tests { } } + /// The token is cookie-only. A query string carrying the *correct* token + /// must not authorize anything — otherwise the secret would still be + /// usable from access logs, `Referer` headers and browser history. #[test] - fn accepts_matching_query_token() { + fn rejects_a_correct_token_in_the_query_string() { let h = HeaderMap::new(); - let a = HeadlessTokenAuthorizer::new(Box::new(DenyAll), "secret".into()); - assert!(a.is_authorized(&ctx(Some("headless=1&token=secret"), &h))); - assert!(a.is_authorized(&ctx(Some("token=secret"), &h))); + let a = HeadlessTokenAuthorizer::new( + Box::new(DenyAll), + "silverbullet_headless_a".into(), + "secret".into(), + ); + assert!(!a.is_authorized(&ctx(Some("headless=1&token=secret"), &h))); + assert!(!a.is_authorized(&ctx(Some("token=secret"), &h))); } #[test] - fn rejects_wrong_or_missing_token() { - let h = HeaderMap::new(); - let a = HeadlessTokenAuthorizer::new(Box::new(DenyAll), "secret".into()); - assert!(!a.is_authorized(&ctx(Some("headless=1&token=wrong"), &h))); - assert!(!a.is_authorized(&ctx(Some("token=secretX"), &h))); - assert!(!a.is_authorized(&ctx(None, &h))); - } - - #[test] - fn empty_token_never_matches_query() { - let h = HeaderMap::new(); - let a = HeadlessTokenAuthorizer::new(Box::new(DenyAll), String::new()); - assert!(!a.is_authorized(&ctx(Some("token="), &h))); + fn rejects_wrong_or_missing_cookie_token() { + let mut wrong = HeaderMap::new(); + wrong.insert( + axum::http::header::COOKIE, + axum::http::HeaderValue::from_static("silverbullet_headless_a=secretX"), + ); + let a = HeadlessTokenAuthorizer::new( + Box::new(DenyAll), + "silverbullet_headless_a".into(), + "secret".into(), + ); + assert!(!a.is_authorized(&ctx(None, &wrong))); + assert!(!a.is_authorized(&ctx(None, &HeaderMap::new()))); } #[test] fn falls_through_to_inner_when_token_absent() { let h = HeaderMap::new(); - let a = HeadlessTokenAuthorizer::new(Box::new(AllowAll), "secret".into()); + let a = HeadlessTokenAuthorizer::new( + Box::new(AllowAll), + "silverbullet_headless_a".into(), + "secret".into(), + ); // No token, but inner allows → authorized. assert!(a.is_authorized(&ctx(None, &h))); } + + #[test] + fn cookie_name_is_per_space() { + assert_eq!( + headless_cookie_name("2f6c1e0a-1111-2222-3333-444455556666"), + "silverbullet_headless_2f6c1e0a-1111-2222-3333-444455556666" + ); + assert_eq!( + headless_cookie_name("single"), + "silverbullet_headless_single" + ); + } + + #[test] + fn accepts_matching_cookie_token() { + let mut h = HeaderMap::new(); + h.insert( + axum::http::header::COOKIE, + axum::http::HeaderValue::from_static("silverbullet_headless_a=secret"), + ); + let a = HeadlessTokenAuthorizer::new( + Box::new(DenyAll), + "silverbullet_headless_a".into(), + "secret".into(), + ); + assert!(a.is_authorized(&ctx(None, &h))); + } + + /// The whole point of the per-space name: space B's cookie, riding along + /// because a root-bound space set `Path=/`, must not authorize space A. + #[test] + fn rejects_another_spaces_cookie() { + let mut h = HeaderMap::new(); + h.insert( + axum::http::header::COOKIE, + axum::http::HeaderValue::from_static( + "silverbullet_headless_b=b-token; silverbullet_headless_a=a-token", + ), + ); + let a = HeadlessTokenAuthorizer::new( + Box::new(DenyAll), + "silverbullet_headless_a".into(), + "b-token".into(), + ); + assert!(!a.is_authorized(&ctx(None, &h))); + } + + #[test] + fn empty_token_never_matches_cookie() { + let mut h = HeaderMap::new(); + h.insert( + axum::http::header::COOKIE, + axum::http::HeaderValue::from_static("silverbullet_headless_a="), + ); + let a = HeadlessTokenAuthorizer::new( + Box::new(DenyAll), + "silverbullet_headless_a".into(), + String::new(), + ); + assert!(!a.is_authorized(&ctx(None, &h))); + } } diff --git a/server/src/auth/mod.rs b/server/src/auth/mod.rs index 27839e1a..d4293b45 100644 --- a/server/src/auth/mod.rs +++ b/server/src/auth/mod.rs @@ -20,7 +20,7 @@ pub use cookie::{ auth_cookie_name, cookie_value, is_secure_request, request_host, scoped_auth_cookie_name, CookieOptions, }; -pub use headless_token::HeadlessTokenAuthorizer; +pub use headless_token::{headless_cookie_name, HeadlessTokenAuthorizer}; pub use jwt_authorizer::JwtAuthorizer; pub use lockout::LockoutTimer; pub use login::LoginManager; diff --git a/server/src/multi/instance.rs b/server/src/multi/instance.rs index 405064c8..2064b71f 100644 --- a/server/src/multi/instance.rs +++ b/server/src/multi/instance.rs @@ -13,8 +13,8 @@ use silverbullet_server_common::space::{ use silverbullet_server_common::{BootConfig, FileMeta, SpaceError, SpacePrimitives}; use crate::auth::{ - AuthConfig, Authenticator, HeadlessTokenAuthorizer, JwtAuthorizer, LockoutTimer, LoginManager, - RequestAuthorizer, + headless_cookie_name, AuthConfig, Authenticator, HeadlessTokenAuthorizer, JwtAuthorizer, + LockoutTimer, LoginManager, RequestAuthorizer, }; use crate::multi::access::{SessionPolicy, SpaceUsersAuth, UserTokenAuthorizer}; use crate::multi::config::{Binding, SpaceConfig}; @@ -32,7 +32,7 @@ pub struct AssetFactories { /// Everything the runtime factory needs to (maybe) build a backend for a space. pub struct RuntimeRequest<'a> { - pub space_folder: &'a str, + pub space_id: &'a str, pub server_url: String, pub headless_token: &'a str, pub read_only: bool, @@ -237,6 +237,7 @@ type AuthPair = ( /// Classic single-space authorizer/login pair: one `AuthConfig` drives both /// the JWT authorizer (with its bearer token) and the login manager. fn build_env_style_auth( + space_id: &str, folder: &Path, prefix: &str, ac: &AuthConfig, @@ -253,6 +254,7 @@ fn build_env_style_auth( )); let authorizer: Arc = Arc::new(HeadlessTokenAuthorizer::new( inner, + headless_cookie_name(space_id), headless_token.to_string(), )); let lockout = LockoutTimer::from_config(ac.lockout_time_secs, ac.lockout_limit); @@ -346,7 +348,7 @@ fn try_build_state( let (authorizer, login): AuthPair = match &deps.auth { InstanceAuth::Single(None) => (None, None), InstanceAuth::Single(Some(config)) => { - build_env_style_auth(&folder, prefix, config, &headless_token)? + build_env_style_auth(id, &folder, prefix, config, &headless_token)? } InstanceAuth::Accounts { .. } if config.public => (None, None), InstanceAuth::Accounts { @@ -366,8 +368,11 @@ fn try_build_state( store.clone(), member_name_filter(store.clone(), members.clone()), )); - let authorizer: Arc = - Arc::new(HeadlessTokenAuthorizer::new(tokens, headless_token.clone())); + let authorizer: Arc = Arc::new(HeadlessTokenAuthorizer::new( + tokens, + headless_cookie_name(id), + headless_token.clone(), + )); let verifier = Arc::new(SpaceUsersAuth { store: store.clone(), members, @@ -406,7 +411,7 @@ fn try_build_state( None } else { (deps.runtime)(&RuntimeRequest { - space_folder: &folder_str, + space_id: id, server_url, headless_token: &headless_token, read_only: config.read_only,