SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(web): unblock session node creation, isolate account switches, persist online tenants

Browse source 
Three review blockers:

- the session push gate refused every id absent from the daemon
  snapshot, which included all ids the namespace allocator mints — so
  nodes created during a session never synced and were erased by the
  next pull. Unknown ids now pass when they parse into this peer's
  namespace (parsed comparison, not prefix match); with no allocator
  the old fail-closed behaviour stands
- a same-tab account switch kept the previous account's document,
  sync baseline, and stored credentials. An identity epoch keyed on
  the auth subject (tri-state, so sign-out then sign-in is not
  mistaken for first contact) resets the sync client, gate, document,
  collab state and latches; 401/403 suppress pushes until the reset;
  browser storage partitions by subject with legacy keys deliberately
  not adopted
- online tenants lived only in memory under the default deploy:
  compose now mounts a data volume, startup fails closed when
  eviction is enabled without a store (OPENPENCIL_ONLINE_EPHEMERAL=1
  is the loud demo bypass), a dedicated sweeper replaces the
  connection-triggered sweep that never ran while idle, and shutdown
  flushes every resident tenant
This commit is contained in:
Kayshen-X 2026-08-08 16:29:41 +08:00
parent 5536018c73
commit f70bf89006
19 changed files with 1038 additions and 195 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -4184,6 +4184,7 @@ dependencies = [
"op-figma",
"op-html",
"op-pen-loader",
"op-util",
"serde",
"serde_json",
"ttf-parser",

136
crates/op-host-services/src/web_canvas_server/online_run_loop.rs
View file

@ -39,13 +39,11 @@ use super::tenant::{now_unix, TenantLimits, TenantRegistry};
use super::tenant_auth::{IdentityVerifier, PresentedCredentials, StaticVerifier};
use super::*;
/// How often the accept loop sweeps for idle tenants.
///
/// Eviction is cheap (a map scan under one lock) and the deadline it enforces
/// is measured in minutes, so a sweep tied to connection arrivals would be
/// both too eager under load and never under idle. This is the floor between
/// sweeps, checked as connections arrive.
const EVICT_SWEEP_INTERVAL_SECS: u64 = 60;
/// Longest gap between idle sweeps, however long the idle deadline is.
const MAX_SWEEP_INTERVAL_SECS: u64 = 300;
/// Opt-in to running with no persistence at all. Demo use only.
pub const EPHEMERAL_ENV: &str = "OPENPENCIL_ONLINE_EPHEMERAL";
/// Run the multi-account web-canvas daemon.
///
@ -63,6 +61,23 @@ pub fn run_online_web_canvas(options: ServeWebOptions) -> Result<()> {
);
}
let limits = TenantLimits::from_env();
let store = super::tenant_store::TenantStore::from_env();
// Eviction without persistence silently destroys documents: a tenant goes
// idle, is reclaimed to free memory, and the account's work is simply
// gone. That is defensible for a demo and indefensible for a deployment,
// and the two are indistinguishable from inside the process — so the
// operator has to say which one this is.
check_persistence_configured(
store.is_enabled(),
ephemeral_opt_in(),
limits.idle_evict_secs,
)?;
if !store.is_enabled() {
eprintln!(
"openpencil --serve-web --online: {EPHEMERAL_ENV} is set — evicted accounts lose \
their documents. Never use this for a deployment."
);
}
let allow_origins = online_policy::allowed_origins_from_env();
if allow_origins.is_empty() {
// Not fatal: a same-origin deployment behind a reverse proxy needs no
@ -100,10 +115,15 @@ pub fn run_online_web_canvas(options: ServeWebOptions) -> Result<()> {
),
}
let registry = Arc::new(TenantRegistry::new(bound, limits, allow_origins));
let registry = Arc::new(TenantRegistry::with_store(
bound,
limits,
allow_origins,
store,
));
let conn_count = Arc::new(AtomicUsize::new(0));
let shutdown = Arc::new(AtomicBool::new(false));
let mut last_sweep = now_unix();
spawn_sweeper(&registry, &shutdown);
for stream in listener.incoming() {
if shutdown.load(Ordering::Acquire) {
@ -116,11 +136,6 @@ pub fn run_online_web_canvas(options: ServeWebOptions) -> Result<()> {
continue;
}
};
let now = now_unix();
if now.saturating_sub(last_sweep) >= EVICT_SWEEP_INTERVAL_SECS {
last_sweep = now;
registry.evict_idle(now);
}
if conn_count.load(Ordering::Acquire) >= limits.max_conns {
let _ = s.set_write_timeout(Some(IO_TIMEOUT));
let _ = crate::mcp_serve::write_mcp_http_response(
@ -154,10 +169,101 @@ pub fn run_online_web_canvas(options: ServeWebOptions) -> Result<()> {
conn_count.fetch_sub(1, Ordering::AcqRel);
}
}
eprintln!("openpencil --serve-web --online: shutdown requested; exiting");
// The sweeper observes the same flag and retires on its next wake.
shutdown.store(true, Ordering::Release);
let flushed = registry.flush_all();
eprintln!(
"openpencil --serve-web --online: shutdown requested; flushed {flushed} account(s); \
exiting"
);
Ok(())
}
/// Refuse to start a deployment that evicts accounts but keeps nothing.
///
/// Pure so the decision is testable without a socket or the environment. The
/// two states this separates — "a demo that intentionally forgets" and "a
/// deployment whose data directory was left unset" — look identical from
/// inside the process, and only one of them is acceptable. So the operator
/// has to say which, rather than the daemon guessing and silently destroying
/// documents on the first idle timer.
pub(super) fn check_persistence_configured(
store_enabled: bool,
ephemeral: bool,
idle_evict_secs: u64,
) -> Result<()> {
if store_enabled || ephemeral {
return Ok(());
}
Err(WebCanvasError::Config(format!(
"--online evicts idle accounts after {idle_evict_secs}s but no {} is configured, so \
their documents would be discarded. Set {} to persist them, or set {EPHEMERAL_ENV}=1 \
to accept that this deployment keeps nothing.",
super::tenant_store::DATA_DIR_ENV,
super::tenant_store::DATA_DIR_ENV,
)))
}
/// Whether the operator accepted a deployment that persists nothing.
fn ephemeral_opt_in() -> bool {
std::env::var(EPHEMERAL_ENV)
.ok()
.map(|value| value.trim().to_string())
.is_some_and(|value| matches!(value.as_str(), "1" | "true" | "yes" | "on"))
}
/// Run the idle sweep on its own clock.
///
/// Eviction used to piggyback on connection arrivals, which meant the one
/// state it exists for — a daemon nobody is talking to — was exactly the state
/// it never ran in: idle accounts stayed resident indefinitely and were never
/// written to disk. This thread observes `shutdown` on every wake, so it
/// retires within one interval of the daemon being asked to stop.
fn spawn_sweeper(registry: &Arc<TenantRegistry>, shutdown: &Arc<AtomicBool>) {
let registry = Arc::clone(registry);
let shutdown = Arc::clone(shutdown);
let interval = sweep_interval_secs(registry.limits().idle_evict_secs);
let spawned = thread::Builder::new()
.name("op-serve-web-online-sweeper".into())
.spawn(move || {
while !shutdown.load(Ordering::Acquire) {
std::thread::sleep(std::time::Duration::from_secs(interval));
if shutdown.load(Ordering::Acquire) {
return;
}
let evicted = registry.evict_idle(now_unix());
if evicted > 0 {
eprintln!(
"openpencil --serve-web --online: reclaimed {evicted} idle account(s)"
);
}
}
});
if spawned.is_err() {
eprintln!(
"openpencil --serve-web --online: could not start the idle sweeper; accounts will \
stay resident until a connection triggers a sweep"
);
}
}
/// How often to sweep, given the idle deadline.
///
/// A quarter of the deadline bounds how long past its timer a tenant can
/// linger, and the ceiling keeps a very long deadline from meaning the daemon
/// effectively never sweeps. The floor keeps a short test deadline from
/// spinning the thread.
pub(super) const fn sweep_interval_secs(idle_evict_secs: u64) -> u64 {
let quarter = idle_evict_secs / 4;
if quarter < 1 {
1
} else if quarter > MAX_SWEEP_INTERVAL_SECS {
MAX_SWEEP_INTERVAL_SECS
} else {
quarter
}
}
/// Pick the identity verifier this deployment runs.
///
/// The hub is the production answer. `StaticVerifier` stays reachable so the

92
crates/op-host-services/src/web_canvas_server/online_share_tests.rs
View file

@ -407,3 +407,95 @@ fn a_tenant_that_cannot_be_written_stays_resident_rather_than_losing_its_documen
let _ = std::fs::remove_file(&temp.root);
}
// ---------------------------------------------------------------------------
// The persistence lifecycle: sweep cadence, shutdown flush, fail-closed start.
// ---------------------------------------------------------------------------
#[test]
fn the_sweep_interval_tracks_the_idle_deadline_within_bounds() {
use crate::web_canvas_server::online_run_loop::sweep_interval_secs;
// A quarter of the deadline bounds how long past its timer a tenant can
// linger — but the production default's quarter (450 s) exceeds the
// ceiling, so it clamps.
assert_eq!(sweep_interval_secs(1800), 300);
assert_eq!(sweep_interval_secs(400), 100);
// …with a ceiling, so a very long deadline still sweeps regularly…
assert_eq!(sweep_interval_secs(86_400), 300);
// …and a floor, so a short test deadline does not spin the thread.
for tiny in [0, 1, 2, 3] {
assert_eq!(sweep_interval_secs(tiny), 1, "{tiny}");
}
}
#[test]
fn an_idle_account_is_reclaimed_without_any_new_connection() {
// The regression: eviction used to run only when a connection arrived, so
// the one state it exists for — an idle daemon — was the state it never
// ran in, and nothing was ever written to disk.
let temp = PersistentRegistry::new("no-traffic");
serve(
&temp.registry,
&verifier(),
Request::json("POST", "/api/mcp/document", SYNC_BODY).with_bearer("tokA"),
);
assert_eq!(temp.registry.tenant_count(), 1);
// No further requests — exactly what the sweeper thread calls.
assert_eq!(temp.registry.evict_idle(now_unix() + 3600), 1);
assert_eq!(temp.registry.tenant_count(), 0);
assert!(temp.registry.store().has_document("userA"));
}
#[test]
fn a_controlled_shutdown_flushes_every_resident_account() {
// Without this, every account active at the moment of a deploy loses
// whatever had not happened to be evicted.
let temp = PersistentRegistry::new("flush");
let verifier = verifier();
for token in ["tokA", "tokB"] {
serve(
&temp.registry,
&verifier,
Request::json("POST", "/api/mcp/document", SYNC_BODY).with_bearer(token),
);
}
assert_eq!(temp.registry.tenant_count(), 2);
assert!(!temp.registry.store().has_document("userA"));
assert_eq!(temp.registry.flush_all(), 2);
assert!(temp.registry.store().has_document("userA"));
assert!(temp.registry.store().has_document("userB"));
// Flushing does not evict: requests still draining must keep working.
assert_eq!(temp.registry.tenant_count(), 2);
}
#[test]
fn flushing_a_deployment_that_persists_nothing_is_a_no_op() {
let registry = registry();
serve(
&registry,
&verifier(),
Request::json("POST", "/api/mcp/document", SYNC_BODY).with_bearer("tokA"),
);
assert_eq!(registry.flush_all(), 0);
}
#[test]
fn a_deployment_that_evicts_but_persists_nothing_refuses_to_start() {
use crate::web_canvas_server::online_run_loop::check_persistence_configured;
// The dangerous default: eviction on, data directory unset. Starting here
// means every idle account's document is destroyed on its first timer.
let refused = check_persistence_configured(false, false, 1800).unwrap_err();
let message = refused.to_string();
assert!(message.contains("OPENPENCIL_ONLINE_DATA_DIR"), "{message}");
assert!(message.contains("OPENPENCIL_ONLINE_EPHEMERAL"), "{message}");
// Configured persistence is the normal deployment.
assert!(check_persistence_configured(true, false, 1800).is_ok());
// An explicit opt-in is the demo, and says so.
assert!(check_persistence_configured(false, true, 1800).is_ok());
// Both is fine — the data directory simply wins.
assert!(check_persistence_configured(true, true, 1800).is_ok());
}

29
crates/op-host-services/src/web_canvas_server/tenant.rs
View file

@ -459,6 +459,35 @@ impl TenantRegistry {
}
}
/// Write every resident tenant to disk.
///
/// Called on controlled shutdown. Eviction is the only other writer, and
/// a daemon that is asked to stop has by definition not waited out anyone's
/// idle timer — so without this, every account that was active at the
/// moment of a deploy loses whatever it had not had evicted.
///
/// Returns how many were written. Tenants are NOT removed: the process is
/// going away regardless, and removing them would only race the requests
/// still draining.
pub fn flush_all(&self) -> usize {
if !self.store.is_enabled() {
return 0;
}
let tenants = self.lock();
let mut written = 0;
for (id, tenant) in tenants.iter() {
let guard = tenant.state.lock().unwrap_or_else(|p| p.into_inner());
match self.store.save(id, &guard.editor, &tenant.shared_with()) {
Ok(()) => written += 1,
Err(error) => eprintln!(
"openpencil --serve-web --online: could not flush a tenant on shutdown \
({error})"
),
}
}
written
}
/// Persist a tenant's access list, if persistence is on.
pub fn persist_acl(&self, user_id: &str, tenant: &Tenant) {
if !self.store.is_enabled() {

4
crates/op-host-web/Cargo.toml
View file

@ -29,6 +29,10 @@ op-editor-ui = { path = "../op-editor-ui" }
# compiled through the unconditional op-editor-ui dep, and the baseline
# `daemon_base` module reads its shared MCP-port/daemon-origin consts.
op-editor-core = { path = "../op-editor-core" }
# The canonical collaboration id grammar. Dependency-free and wasm32-clean —
# the push gate parses namespaced ids with it rather than matching prefixes.
op-util = { path = "../op-util" }
# These stay optional and are pulled only by `canvaskit`, so the wasm32-clean
# stub baseline (`--no-default-features --features web`) never compiles the
# full editor pipeline. `op-pen-loader` disables `skia-measure` below; the web

110
crates/op-host-web/src/collab_sync.rs
View file

@ -75,7 +75,16 @@ thread_local! {
///
/// The browser mints ids from a local sequential counter, which is exactly
/// what an active session cannot accept — see [`push_blocked_by_session`].
static DAEMON_NODE_IDS: RefCell<Option<HashSet<op_editor_core::NodeId>>> =
pub(super) static DAEMON_NODE_IDS: RefCell<Option<HashSet<op_editor_core::NodeId>>> =
const { RefCell::new(None) };
/// The namespace this peer is currently minting ids under, when the
/// owner-assigned allocator is enabled.
///
/// Set by `sync_id_allocation` at the moment the allocator is installed
/// and cleared the moment it is taken away, so it is exactly "the ids
/// this peer is entitled to invent" — see `push_blocked_by_session`.
pub(super) static SESSION_NAMESPACE: RefCell<Option<op_editor_core::PeerNamespace>> =
const { RefCell::new(None) };
}
@ -95,42 +104,11 @@ pub(crate) fn note_daemon_document(state: &op_editor_core::EditorState) {
DAEMON_NODE_IDS.with(|ids| *ids.borrow_mut() = Some(document_node_ids(state)));
}
/// Whether a live session forbids pushing the current local document.
///
/// The browser has no owner-assigned id namespace: it mints `n<counter>` from a
/// local sequential allocator, and two peers creating a node in the same moment
/// would mint the same id. The collaboration protocol replays those ids
/// verbatim, so a colliding pair silently forks the document — the failure this
/// refuses to produce.
///
/// The check is deliberately whole-document rather than per-gesture: draw,
/// duplicate, paste, group and import all mint through the same counter, and
/// gating the single push covers every one of them without a guard at each
/// call site. Any id the daemon has not seen blocks the push; edits to existing
/// nodes (move, restyle, delete) carry no new ids and go through untouched.
///
/// The local node stays on screen until the next pull replaces it with the
/// daemon's document, so the divergence is bounded and self-healing.
pub(crate) fn push_blocked_by_session(state: &op_editor_core::EditorState) -> bool {
if state.editor_ui.collab.phase != CollabConnectionPhase::Active {
return false;
}
DAEMON_NODE_IDS.with(|known| {
let known = known.borrow();
let Some(known) = known.as_ref() else {
// No daemon document seen yet in this session; refuse rather than
// guess, since the pull that would settle it is one tick away.
return true;
};
document_node_ids(state)
.iter()
.any(|id| !known.contains(id))
})
}
/// Node ids in a document, through `op-editor-core`'s own walker so pages and
/// nodes share the one collision domain the allocator uses.
fn document_node_ids(state: &op_editor_core::EditorState) -> HashSet<op_editor_core::NodeId> {
pub(super) fn document_node_ids(
state: &op_editor_core::EditorState,
) -> HashSet<op_editor_core::NodeId> {
op_editor_core::collect_document_ids(&state.doc)
}
@ -363,15 +341,21 @@ fn sync_id_allocation(host: &mut crate::widget_host::WidgetHost, wire: &CollabSt
Some(namespace) if !host.collaboration_ids_enabled() => {
match op_editor_core::PeerNamespace::parse(namespace) {
Ok(namespace) => {
let enabled = namespace.clone();
if let Err(error) = host.enable_collaboration_ids(namespace) {
// The document already carries ids this namespace
// cannot resume above. Staying on the standalone
// counter keeps the canvas usable; the push gate is
// what keeps those ids off the wire.
let _ = error;
} else {
set_session_namespace(Some(enabled));
}
}
Err(_) => host.disable_collaboration_ids(),
Err(_) => {
host.disable_collaboration_ids();
set_session_namespace(None);
}
}
}
Some(_) => {}
@ -379,10 +363,28 @@ fn sync_id_allocation(host: &mut crate::widget_host::WidgetHost, wire: &CollabSt
if host.collaboration_ids_enabled() {
host.disable_collaboration_ids();
}
set_session_namespace(None);
}
}
}
/// Forget everything scoped to the previous account.
///
/// The daemon snapshot and the minting namespace both belong to the session
/// the old account was in; carrying them into a new account's tab would let
/// its push gate answer from another account's document.
pub(crate) fn reset_for_new_identity() {
DAEMON_NODE_IDS.with(|ids| *ids.borrow_mut() = None);
SESSION_NAMESPACE.with(|slot| *slot.borrow_mut() = None);
APPLIED_SEQ.set(None);
SESSION_LIVE.set(false);
}
/// Record (or clear) the namespace this peer mints under.
fn set_session_namespace(namespace: Option<op_editor_core::PeerNamespace>) {
SESSION_NAMESPACE.with(|slot| *slot.borrow_mut() = namespace);
}
fn maybe_push_presence<C: RepaintContext + 'static>(inner: &Rc<RefCell<C>>, base: &str) {
if !SESSION_LIVE.get() {
return;
@ -428,7 +430,7 @@ fn now_ms() -> u64 {
mod tests {
use super::*;
fn doc_with(ids: &[&str]) -> op_editor_core::EditorState {
pub(super) fn doc_with(ids: &[&str]) -> op_editor_core::EditorState {
let children: Vec<serde_json::Value> = ids
.iter()
.map(|id| {
@ -447,8 +449,13 @@ mod tests {
state
}
fn reset_latches() {
pub(super) fn reset_latches() {
DAEMON_NODE_IDS.with(|ids| *ids.borrow_mut() = None);
SESSION_NAMESPACE.with(|slot| *slot.borrow_mut() = None);
}
pub(super) fn namespace(value: &str) -> op_editor_core::PeerNamespace {
op_editor_core::PeerNamespace::parse(value.to_string()).expect("valid namespace")
}
#[test]
@ -466,29 +473,6 @@ mod tests {
);
}
#[test]
fn an_active_session_blocks_a_push_that_invents_a_node_id() {
reset_latches();
let mut state = doc_with(&["n100"]);
note_daemon_document(&state);
state
.editor_ui
.collab
.set_phase(CollabConnectionPhase::Active);
// Editing what the daemon already knows is fine.
assert!(!push_blocked_by_session(&state));
// Minting a new local id is not: the browser has no owner-assigned
// namespace, so this id could collide with a peer's.
let mut grown = doc_with(&["n100", "n101"]);
grown
.editor_ui
.collab
.set_phase(CollabConnectionPhase::Active);
assert!(push_blocked_by_session(&grown));
}
#[test]
fn deleting_a_node_during_a_session_is_not_blocked() {
reset_latches();
@ -690,3 +674,7 @@ fn note_stream_failure() {
));
}
}
#[path = "collab_sync_push_gate.rs"]
mod push_gate;
pub(crate) use push_gate::push_blocked_by_session;

179
crates/op-host-web/src/collab_sync_push_gate.rs Normal file
View file

@ -0,0 +1,179 @@
//! The session push gate: which local document may reach the daemon.
//!
//! Split out of `collab_sync.rs` at the 800-line cap. It is one decision, and
//! the decision is load-bearing: too strict and every node created during a
//! session is stuck forever (and deleted by the next pull); too loose and this
//! peer forks the shared document with ids another peer may also mint.
use op_editor_core::CollabConnectionPhase;
use super::{document_node_ids, DAEMON_NODE_IDS, SESSION_NAMESPACE};
/// Whether a live session forbids pushing the current local document.
///
/// The browser has no owner-assigned id namespace: it mints `n<counter>` from a
/// local sequential allocator, and two peers creating a node in the same moment
/// would mint the same id. The collaboration protocol replays those ids
/// verbatim, so a colliding pair silently forks the document — the failure this
/// refuses to produce.
///
/// The check is deliberately whole-document rather than per-gesture: draw,
/// duplicate, paste, group and import all mint through the same counter, and
/// gating the single push covers every one of them without a guard at each
/// call site. Edits to existing nodes (move, restyle, delete) carry no new ids
/// and go through untouched.
///
/// ## Which unknown ids are allowed
///
/// An id the daemon has not seen is only dangerous when this peer had no
/// right to invent it. Once the owner grants a namespace and the allocator is
/// installed, `c_<namespace>_<counter>` ids ARE this peer's to mint — the
/// namespace is what makes them collision-free — so blocking them would mean
/// every node created during a session is stuck forever, and the next pull
/// would quietly delete it. So the gate refuses an id only when it is both
/// unknown to the daemon AND outside this peer's namespace.
///
/// With no namespace (an older daemon, or a session that has not reached
/// `Active`) the allocator is not installed and every unknown id is refused,
/// exactly as before: the browser is minting bare `n<counter>` ids that could
/// collide with a peer's.
///
/// The local node stays on screen until the next pull replaces it with the
/// daemon's document, so the divergence is bounded and self-healing.
pub(crate) fn push_blocked_by_session(state: &op_editor_core::EditorState) -> bool {
if state.editor_ui.collab.phase != CollabConnectionPhase::Active {
return false;
}
DAEMON_NODE_IDS.with(|known| {
let known = known.borrow();
let Some(known) = known.as_ref() else {
// No daemon document seen yet in this session; refuse rather than
// guess, since the pull that would settle it is one tick away.
return true;
};
SESSION_NAMESPACE.with(|namespace| {
let namespace = namespace.borrow();
document_node_ids(state)
.iter()
.any(|id| !known.contains(id) && !minted_by_this_peer(id, namespace.as_ref()))
})
})
}
/// Whether `id` is one this peer was entitled to mint.
///
/// Parsed through the shared `op-util` grammar rather than a hand-rolled
/// prefix test: `c_`/`_` are structural, a namespace has its own character
/// rules, and a substring check would accept `c_teamA-evil_7` as belonging to
/// `teamA`. Comparing parsed namespaces is what makes that impossible.
fn minted_by_this_peer(
id: &op_editor_core::NodeId,
namespace: Option<&op_editor_core::PeerNamespace>,
) -> bool {
let Some(namespace) = namespace else {
return false;
};
op_util::collab_id::NamespacedId::parse(id.as_str())
.is_ok_and(|parsed| parsed.namespace() == namespace)
}
#[cfg(test)]
mod tests {
use super::super::note_daemon_document;
use super::super::tests::{doc_with, namespace, reset_latches};
use super::*;
#[test]
fn an_active_session_blocks_an_id_this_peer_had_no_right_to_invent() {
reset_latches();
let mut state = doc_with(&["n100"]);
note_daemon_document(&state);
state
.editor_ui
.collab
.set_phase(CollabConnectionPhase::Active);
// Editing what the daemon already knows is fine.
assert!(!push_blocked_by_session(&state));
// Minting a bare local id is not: with no owner-assigned namespace
// installed, this id could collide with a peer's.
let mut grown = doc_with(&["n100", "n101"]);
grown
.editor_ui
.collab
.set_phase(CollabConnectionPhase::Active);
assert!(push_blocked_by_session(&grown));
}
#[test]
fn a_namespaced_id_this_peer_minted_is_pushed() {
// The regression this replaces: every node created during a session
// was blocked forever, because an id the peer had just been granted
// the right to mint is by definition not in the daemon's snapshot.
reset_latches();
let seed = doc_with(&["n100"]);
note_daemon_document(&seed);
SESSION_NAMESPACE.with(|slot| *slot.borrow_mut() = Some(namespace("teamA")));
for created in [
// create, duplicate and paste all mint through the same counter.
vec!["n100", "c_teamA_1"],
vec!["n100", "c_teamA_1", "c_teamA_2"],
vec!["n100", "c_teamA_4294967296"],
] {
let mut state = doc_with(&created);
state
.editor_ui
.collab
.set_phase(CollabConnectionPhase::Active);
assert!(
!push_blocked_by_session(&state),
"a node this peer was entitled to create must reach the daemon: {created:?}"
);
}
}
#[test]
fn an_id_from_another_peers_namespace_is_still_blocked() {
reset_latches();
let seed = doc_with(&["n100"]);
note_daemon_document(&seed);
SESSION_NAMESPACE.with(|slot| *slot.borrow_mut() = Some(namespace("teamA")));
for hostile in [
"c_teamB_1", // another peer's namespace
"c_teamA-evil_1", // a substring match a prefix test would accept
"c_teamAevil_1", // ditto, no separator
"n101", // a bare local id
"c_teamA", // no counter
"teamA_1", // no prefix
] {
let mut state = doc_with(&["n100", hostile]);
state
.editor_ui
.collab
.set_phase(CollabConnectionPhase::Active);
assert!(
push_blocked_by_session(&state),
"{hostile} is not this peer's to mint"
);
}
}
#[test]
fn a_namespaced_id_is_blocked_when_no_allocator_is_installed() {
// Fail-closed: without the allocator the peer is minting bare ids, so
// a namespaced-looking id in the document was not minted here.
reset_latches();
let seed = doc_with(&["n100"]);
note_daemon_document(&seed);
let mut state = doc_with(&["n100", "c_teamA_1"]);
state
.editor_ui
.collab
.set_phase(CollabConnectionPhase::Active);
assert!(push_blocked_by_session(&state));
}
}

265
crates/op-host-web/src/identity_epoch.rs Normal file
View file

@ -0,0 +1,265 @@
//! Which account this tab currently belongs to, and what happens when that
//! changes.
//!
//! A browser tab outlives a sign-in. Sign out of A and into B in the same tab
//! and, without this, B inherits A's document (the shell keeps whatever was
//! last applied), A's provider credentials (same-origin storage key), and A's
//! sync baseline — and B's own document, starting at a LOWER daemon version,
//! cannot displace it, because the sync client only accepts a version higher
//! than the one it has applied. So the leak is not transient; it is what the
//! tab shows until it is reloaded.
//!
//! The fix is an epoch. Every `/api/auth/status` answer carries a subject; a
//! change of subject — including signed-in → signed-out → signed-in-as-someone
//! -else — bumps the epoch, and everything keyed to an account is dropped.
//!
//! ## Why the subject and not the display name
//!
//! `username` is the account handle the daemon reports for the session.
//! `display_name` is user-editable and `primary_email` can be absent, so
//! neither identifies an account across a switch.
use std::cell::RefCell;
/// Storage partition used before anyone signs in.
///
/// A real subject can never collide with it: the partition key is prefixed,
/// and this value is not a legal account handle.
pub const ANONYMOUS_SUBJECT: &str = "anon";
thread_local! {
/// The subject this tab is currently showing.
///
/// Three states, and the distinction matters: the outer `None` means no
/// status answer has arrived yet, `Some(None)` means observed and signed
/// out, and `Some(Some(subject))` means signed in. Collapsing the first
/// two would make a sign-in after a sign-out look like a tab's very first
/// answer — and that is exactly the A → out → B switch this exists to
/// catch.
static SUBJECT: RefCell<Option<Option<String>>> = const { RefCell::new(None) };
/// Bumped on every observed identity change.
static EPOCH: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
}
/// The account partition in force, for storage keys.
pub fn current_subject() -> String {
SUBJECT.with(|slot| {
slot.borrow()
.clone()
.flatten()
.unwrap_or_else(|| ANONYMOUS_SUBJECT.to_string())
})
}
/// How many identity changes this tab has seen.
pub fn epoch() -> u64 {
EPOCH.with(std::cell::Cell::get)
}
/// Record the subject an `/api/auth/status` answer reported.
///
/// Returns `true` when the identity CHANGED and the caller must therefore
/// drop everything keyed to the previous account. The first observation is
/// not a change: the tab has shown nothing yet.
pub fn observe_subject(subject: Option<&str>) -> bool {
let next = subject
.map(str::trim)
.filter(|value| !value.is_empty())
.map(str::to_string);
SUBJECT.with(|slot| {
let mut slot = slot.borrow_mut();
let previous = slot.replace(next.clone());
match previous {
// The tab's first status answer. Whatever it says is what this
// tab has always been showing, so there is nothing to discard —
// but the storage partition is now known, so the epoch moves.
None => {
EPOCH.with(|epoch| epoch.set(epoch.get().saturating_add(1)));
false
}
// A repeat of the same answer: the common case, and it must not
// churn state or every poll would reset the tab.
Some(before) if before == next => false,
// A genuine change — sign-in, sign-out, or a switch between two
// accounts. All three must drop the previous account's state.
Some(_) => {
EPOCH.with(|epoch| epoch.set(epoch.get().saturating_add(1)));
true
}
}
})
}
/// Read the account subject out of an `/api/auth/status` body.
///
/// `None` for a signed-out or unparseable answer, which is the anonymous
/// partition — the safe direction, since it shares nothing with a real one.
pub fn subject_from_status(body: &str) -> Option<String> {
let parsed: serde_json::Value = serde_json::from_str(body).ok()?;
if !parsed["signed_in"].as_bool().unwrap_or(false) {
return None;
}
parsed["username"]
.as_str()
.map(str::trim)
.filter(|value| !value.is_empty())
.map(str::to_string)
}
/// Reset the epoch state. Tests only — a tab never goes back to "no answer".
#[cfg(test)]
pub fn reset_for_test() {
SUBJECT.with(|slot| *slot.borrow_mut() = None);
EPOCH.with(|epoch| epoch.set(0));
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn the_first_anonymous_observation_is_not_a_change() {
reset_for_test();
assert!(!observe_subject(None));
assert_eq!(current_subject(), ANONYMOUS_SUBJECT);
}
#[test]
fn the_first_answer_of_a_tab_is_never_a_reset() {
reset_for_test();
// Whatever the first answer says is what this tab has always shown.
assert!(!observe_subject(Some("alice")));
assert_eq!(current_subject(), "alice");
assert!(epoch() > 0);
}
#[test]
fn signing_in_after_an_observed_sign_out_is_a_reset() {
// Distinct from the case above: the tab HAS shown the anonymous
// state, so signing in replaces what was on screen.
reset_for_test();
assert!(!observe_subject(None));
assert!(observe_subject(Some("alice")));
assert_eq!(current_subject(), "alice");
}
#[test]
fn switching_accounts_reports_a_change() {
reset_for_test();
observe_subject(Some("alice"));
let before = epoch();
assert!(
observe_subject(Some("bob")),
"a different account must reset the tab"
);
assert_eq!(current_subject(), "bob");
assert!(epoch() > before);
}
#[test]
fn signing_out_reports_a_change() {
reset_for_test();
observe_subject(Some("alice"));
assert!(observe_subject(None));
assert_eq!(current_subject(), ANONYMOUS_SUBJECT);
}
#[test]
fn the_sign_out_then_in_path_is_a_change_at_each_step() {
// The leak this exists for: A → anonymous → B in one tab.
reset_for_test();
observe_subject(Some("alice"));
assert!(observe_subject(None));
assert!(observe_subject(Some("bob")));
assert_eq!(current_subject(), "bob");
}
#[test]
fn the_same_account_repeated_is_never_a_change() {
reset_for_test();
observe_subject(Some("alice"));
let epoch_after_sign_in = epoch();
for _ in 0..5 {
assert!(!observe_subject(Some("alice")));
}
assert_eq!(epoch(), epoch_after_sign_in, "a poll must not churn state");
}
#[test]
fn a_subject_is_read_out_of_a_signed_in_status_body() {
assert_eq!(
subject_from_status(r#"{"signed_in":true,"username":"alice"}"#).as_deref(),
Some("alice")
);
}
#[test]
fn a_signed_out_or_unusable_status_body_has_no_subject() {
for body in [
r#"{"signed_in":false,"username":"alice"}"#,
r#"{"signed_in":true}"#,
r#"{"signed_in":true,"username":" "}"#,
"not json",
"{}",
] {
assert_eq!(subject_from_status(body), None, "{body:?}");
}
}
#[test]
fn whitespace_around_a_subject_does_not_create_a_second_partition() {
reset_for_test();
observe_subject(Some("alice"));
assert!(!observe_subject(Some(" alice ")));
}
}
// The storage partitions only exist in the build that has a settings store.
#[cfg(all(test, feature = "canvaskit"))]
mod partition_tests {
use super::*;
#[test]
fn two_accounts_get_two_storage_partitions() {
reset_for_test();
observe_subject(Some("alice"));
let alice_settings = crate::web_settings::settings_storage_key();
let alice_credentials = crate::web_settings::credential_storage_key();
observe_subject(Some("bob"));
let bob_settings = crate::web_settings::settings_storage_key();
let bob_credentials = crate::web_settings::credential_storage_key();
assert_ne!(
alice_settings, bob_settings,
"two accounts sharing a browser must not share a settings blob"
);
assert_ne!(
alice_credentials, bob_credentials,
"one account's provider API keys must not be readable by the next"
);
}
#[test]
fn signing_out_returns_to_the_anonymous_partition() {
reset_for_test();
observe_subject(Some("alice"));
let signed_in = crate::web_settings::credential_storage_key();
observe_subject(None);
let anonymous = crate::web_settings::credential_storage_key();
assert_ne!(signed_in, anonymous);
assert!(anonymous.ends_with(ANONYMOUS_SUBJECT), "{anonymous}");
}
#[test]
fn a_partition_key_is_never_the_bare_legacy_key() {
// The unpartitioned keys may hold a different account's credentials
// than the one now signed in, so they are never read.
reset_for_test();
for subject in [None, Some("alice")] {
observe_subject(subject);
assert!(crate::web_settings::settings_storage_key().contains("::"));
assert!(crate::web_settings::credential_storage_key().contains("::"));
}
}
}

4
crates/op-host-web/src/lib.rs
View file

@ -66,6 +66,10 @@ mod document_json;
mod dom_io;
#[cfg(feature = "canvaskit")]
mod file_actions;
// Which account this tab belongs to. Lives with the daemon-facing modules
// because it is driven by the device-login status poll.
#[cfg(feature = "canvaskit")]
pub mod identity_epoch;
#[cfg(all(test, feature = "canvaskit"))]
mod prompt_center_file_actions_tests;
// Short-lived Worker-side Figma converter. The exported class is instantiated

20
crates/op-host-web/src/live_sync_conflict.rs
View file

@ -217,3 +217,23 @@ mod server_authority_tests {
}
}
}
#[cfg(test)]
mod auth_latch_tests {
use crate::live_sync_glue::{auth_is_invalid, clear_auth_invalid, note_auth_invalid};
#[test]
fn a_refused_credential_stops_the_tab_pushing() {
clear_auth_invalid();
assert!(!auth_is_invalid(), "a healthy tab pushes");
// The document on screen belongs to whoever WAS signed in; pushing it
// after a switch would write one account's work into another's tenant.
note_auth_invalid();
assert!(auth_is_invalid());
// Only the identity reset lifts it, once the tab has been rebuilt.
clear_auth_invalid();
assert!(!auth_is_invalid());
}
}

87
crates/op-host-web/src/live_sync_controller.rs Normal file
View file

@ -0,0 +1,87 @@
//! The per-editor sync controller and the identity pairs every gating
//! decision is keyed on.
//!
//! Split out of `live_sync_glue.rs` at the 800-line cap — pure code motion.
use std::cell::RefCell;
use std::rc::{Rc, Weak};
use op_editor_core::sync_gate::SyncGate;
use op_editor_core::web_sync::WebSyncClient;
use crate::repaint_ctx::RepaintContext;
use super::ACTIVE_SYNC;
/// Shared sync state for one mounted editor.
///
/// The gate and the client must see the same document identity, so they live
/// in one struct rather than as two independently-owned pieces.
pub(crate) struct SyncController {
pub gate: SyncGate,
pub client: WebSyncClient,
pub push_busy: bool,
/// A document identity already measured above the periodic push limit.
/// WASM linear memory does not shrink after a giant temporary JSON string,
/// so do not rebuild the same oversized snapshot every two seconds.
pub(super) oversize_identity: Option<(u64, u64, u64)>,
}
impl SyncController {
pub(crate) fn new() -> Self {
Self {
gate: SyncGate::default(),
client: WebSyncClient::new(),
push_busy: false,
oversize_identity: None,
}
}
}
pub(crate) type SharedSync = Rc<RefCell<SyncController>>;
/// Commit a successful daemon Save as a sync acknowledgement.
///
/// The daemon has already installed exactly the saved snapshot. Recording its
/// version prevents the next probe from downloading and replacing the same
/// potentially huge document, while the snapshot pair reopens the pull gate.
/// A later local edit still differs from this pair and remains eligible for a
/// normal push (or another explicit Save for oversized documents).
pub(crate) fn acknowledge_daemon_save(
version: u64,
generation: u64,
revision: u64,
active_page_index: usize,
preserve_authored_geometry: bool,
) {
ACTIVE_SYNC.with(|slot| {
let Some(sync) = slot.borrow().as_ref().and_then(Weak::upgrade) else {
return;
};
let mut sync = sync.borrow_mut();
sync.client.mark_applied(version);
sync.client
.note_applied_snapshot_without_hash(active_page_index, preserve_authored_geometry);
sync.gate.note_synced(generation, revision);
});
}
/// The document-identity pair every gating decision is keyed on. Read fresh
/// from the live editor state at each decision point — never cached — so an
/// edit that lands between a tick firing and its async response landing is
/// always observed.
pub(super) fn current_pair<C: RepaintContext>(b: &C) -> (u64, u64) {
let s = b.host().editor_state();
(s.document_generation(), s.document_revision())
}
pub(super) fn current_oversize_identity<C: RepaintContext>(b: &C) -> (u64, u64, u64) {
let host = b.host();
let state = host.editor_state();
let doc = state;
(
host.document_epoch(),
doc.document_generation(),
doc.document_revision(),
)
}

125
crates/op-host-web/src/live_sync_glue.rs
View file

@ -120,83 +120,7 @@ fn serialize_sync_document(doc: &op_editor_core::PenDocument) -> SyncDocumentJso
/// once in `mount_ck` and handed to both this module's ticks and the
/// postMessage bridge, so both sides observe and mutate the exact same
/// `SyncGate` instance (a v1 defect — the bridge couldn't reach a local
/// `WebSyncClient` — is fixed by sharing this one struct).
pub(crate) struct SyncController {
pub gate: SyncGate,
pub client: WebSyncClient,
pub push_busy: bool,
/// A document identity already measured above the periodic push limit.
/// WASM linear memory does not shrink after a giant temporary JSON string,
/// so do not rebuild the same oversized snapshot every two seconds.
oversize_identity: Option<(u64, u64, u64)>,
}
impl SyncController {
pub(crate) fn new() -> Self {
Self {
gate: SyncGate::default(),
client: WebSyncClient::new(),
push_busy: false,
oversize_identity: None,
}
}
}
pub(crate) type SharedSync = Rc<RefCell<SyncController>>;
thread_local! {
/// The mounted editor's sync controller, exposed weakly so File → Save can
/// consume the daemon's returned version without creating an ownership
/// cycle or threading sync state through every DOM file-action callback.
static ACTIVE_SYNC: RefCell<Option<Weak<RefCell<SyncController>>>> = const { RefCell::new(None) };
}
/// Commit a successful daemon Save as a sync acknowledgement.
///
/// The daemon has already installed exactly the saved snapshot. Recording its
/// version prevents the next probe from downloading and replacing the same
/// potentially huge document, while the snapshot pair reopens the pull gate.
/// A later local edit still differs from this pair and remains eligible for a
/// normal push (or another explicit Save for oversized documents).
pub(crate) fn acknowledge_daemon_save(
version: u64,
generation: u64,
revision: u64,
active_page_index: usize,
preserve_authored_geometry: bool,
) {
ACTIVE_SYNC.with(|slot| {
let Some(sync) = slot.borrow().as_ref().and_then(Weak::upgrade) else {
return;
};
let mut sync = sync.borrow_mut();
sync.client.mark_applied(version);
sync.client
.note_applied_snapshot_without_hash(active_page_index, preserve_authored_geometry);
sync.gate.note_synced(generation, revision);
});
}
/// The document-identity pair every gating decision is keyed on. Read fresh
/// from the live editor state at each decision point — never cached — so an
/// edit that lands between a tick firing and its async response landing is
/// always observed.
fn current_pair<C: RepaintContext>(b: &C) -> (u64, u64) {
let s = b.host().editor_state();
(s.document_generation(), s.document_revision())
}
fn current_oversize_identity<C: RepaintContext>(b: &C) -> (u64, u64, u64) {
let host = b.host();
let state = host.editor_state();
(
host.document_epoch(),
state.document_generation(),
state.document_revision(),
)
}
/// Content changes advance the gate's `(generation, revision)` pair, while
//// Content changes advance the gate's `(generation, revision)` pair, while
/// active-page switches intentionally do not. Treat an editor-metadata delta
/// as an additional push reason without weakening the conflict latch.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@ -310,7 +234,15 @@ fn poll_version<C: RepaintContext + 'static>(
let base_owned = base.to_string();
let fetch_busy = fetch_busy.clone();
let last_selection_key = last_selection_key.clone();
let on_version: Rc<dyn Fn(String)> = Rc::new(move |body: String| {
let on_version: Rc<dyn Fn(u16, String)> = Rc::new(move |status: u16, body: String| {
// A refused credential is the signal that this tab no longer speaks
// for the account whose document it is showing. Treating it as "no
// version" — which is what an untyped body read does — is what let the
// previous account's document sit on screen indefinitely.
if matches!(status, 401 | 403) {
note_auth_invalid();
return;
}
// The daemon answers both counters here. Hand the collaboration one to
// its own loop rather than opening a second probe; a `collabSeq` bump
// must never reach the document fetch below.
@ -340,7 +272,7 @@ fn poll_version<C: RepaintContext + 'static>(
fetch_busy.set(false);
}
});
let _ = live_sync::get(&format!("{base}/api/mcp/version"), on_version);
let _ = live_sync::get_with_status(&format!("{base}/api/mcp/version"), on_version);
}
/// Auto-resolve a latched push conflict, but only inside a live session.
@ -583,6 +515,13 @@ fn push_document_if_changed<C: RepaintContext + 'static>(
// from the scalar baseline, not mistaken for a content change.
reasons.editor_meta
};
// The daemon refused this tab's credential. Whatever is on screen belongs
// to whoever was signed in before, so pushing it now would write one
// account's work into whichever tenant the new credential resolves to.
// Held until the identity reset rebuilds the tab.
if auth_is_invalid() {
return;
}
// An active collaboration session cannot sequence a node id this browser
// minted from its local counter. Hold the push rather than fork the shared
// document; the next pull replaces the local node with the daemon's copy.
@ -770,6 +709,34 @@ mod tests {
}
}
thread_local! {
/// Raised when the daemon answered a sync request 401/403.
///
/// A tab whose credential stopped being accepted must stop pushing: the
/// document on screen belongs to whoever WAS signed in, and pushing it
/// after a switch would write one account's work into another's tenant.
/// Cleared by [`clear_auth_invalid`], which the identity reset calls once
/// the tab has been rebuilt for the new account.
pub(super) static AUTH_INVALID: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
/// The mounted editor's sync controller, exposed weakly so File → Save can
/// consume the daemon's returned version without creating an ownership
/// cycle or threading sync state through every DOM file-action callback.
pub(super) static ACTIVE_SYNC: RefCell<Option<Weak<RefCell<SyncController>>>> = const { RefCell::new(None) };
}
#[path = "live_sync_controller.rs"]
mod live_sync_controller;
pub(crate) use live_sync_controller::{acknowledge_daemon_save, SharedSync, SyncController};
// Spine-local: the two identity pairs every gating decision here is keyed on.
use live_sync_controller::{current_oversize_identity, current_pair};
#[path = "live_sync_conflict.rs"]
mod live_sync_conflict;
#[path = "live_sync_identity.rs"]
mod live_sync_identity;
pub(crate) use live_sync_identity::{auth_is_invalid, note_auth_invalid, reset_for_new_identity};
// Only the latch's own test lifts it; the reset clears it internally.
use live_sync_conflict::{auto_resolve_is_safe, probe_serve_mode, server_is_authoritative};
#[cfg(test)]
pub(crate) use live_sync_identity::clear_auth_invalid;

67
crates/op-host-web/src/live_sync_identity.rs Normal file
View file

@ -0,0 +1,67 @@
//! What happens when the tab stops speaking for the account it is showing.
//!
//! Split out of `live_sync_glue.rs` at the 800-line cap. Two related things
//! live here: the latch that stops a tab pushing once the daemon has refused
//! its credential, and the reset that rebuilds the tab when
//! `/api/auth/status` reports a different account.
//!
//! Together they close the same hole — a browser tab outlives a sign-in, so
//! without them account B inherits account A's document, sync baseline and
//! session state, and cannot displace them.
use std::cell::RefCell;
use std::rc::{Rc, Weak};
use op_editor_core::sync_gate::SyncGate;
use op_editor_core::web_sync::WebSyncClient;
use crate::repaint_ctx::RepaintContext;
use super::{ACTIVE_SYNC, AUTH_INVALID};
/// Whether the daemon has refused this tab's credential.
pub(crate) fn auth_is_invalid() -> bool {
AUTH_INVALID.with(std::cell::Cell::get)
}
/// Latch the tab out of pushing after a 401/403.
pub(crate) fn note_auth_invalid() {
AUTH_INVALID.with(|flag| flag.set(true));
}
/// Release the latch. Only the identity reset calls this.
pub(crate) fn clear_auth_invalid() {
AUTH_INVALID.with(|flag| flag.set(false));
}
/// Drop everything this tab was showing for the previous account.
///
/// Called when `/api/auth/status` reports a different subject. Without it the
/// tab keeps the previous account's document (the shell simply never replaced
/// it), its sync baseline, and — because the sync client only accepts a
/// version HIGHER than the one it applied — the new account's own document,
/// which starts lower, can never displace it. So this is not tidy-up; it is
/// the only thing that makes the switch take effect at all.
pub(crate) fn reset_for_new_identity<C: RepaintContext + 'static>(inner: &Rc<RefCell<C>>) {
ACTIVE_SYNC.with(|slot| {
if let Some(sync) = slot.borrow().as_ref().and_then(Weak::upgrade) {
let mut sync = sync.borrow_mut();
// A fresh client has applied no version, so the next probe
// downloads the new account's document whatever its number.
sync.client = WebSyncClient::new();
sync.gate = SyncGate::default();
sync.push_busy = false;
}
});
if let Ok(mut context) = inner.try_borrow_mut() {
let state = context.host_mut().editor_state_mut();
// Back to the same starter a fresh tab paints, so nothing of the
// previous account survives on screen.
state.replace_document(op_editor_core::EditorState::starter().doc);
state.editor_ui.collab = op_editor_core::CollabUiState::default();
context.host_mut().mark_editor_state_dirty();
let _ = context.repaint();
}
crate::collab_sync::reset_for_new_identity();
clear_auth_invalid();
}

7
crates/op-host-web/src/web_auth_sync.rs
View file

@ -175,6 +175,13 @@ fn fetch_status<C: RepaintContext + 'static>(inner: &Rc<RefCell<C>>, base: &str)
let Ok(parsed) = serde_json::from_str::<serde_json::Value>(&body) else {
return;
};
// Identity first: everything below paints for an account, so the
// account has to be settled before any of it runs.
if crate::identity_epoch::observe_subject(
crate::identity_epoch::subject_from_status(&body).as_deref(),
) {
crate::live_sync_glue::reset_for_new_identity(&inner);
}
sync_account_avatar(&inner, parsed["avatar_revision"].as_str());
let mut b = inner.borrow_mut();
let ui = &mut b.host_mut().editor_state_mut().editor_ui;

25
crates/op-host-web/src/web_settings.rs
View file

@ -30,6 +30,31 @@ const CREDENTIAL_PAYLOAD_VERSION: u32 = 2;
const STORAGE_KEY: &str = "openpencil-rust-web-settings";
const CREDENTIAL_STORAGE_KEY: &str = "openpencil-rust-web-credentials";
/// Per-account storage keys.
///
/// The base keys above are same-origin and carry no account dimension, so two
/// accounts using one browser shared one settings blob — and one account's
/// provider API keys were readable by the next. Every read and write goes
/// through these instead, partitioned by the signed-in subject
/// (`identity_epoch::current_subject`, `"anon"` before sign-in).
///
/// The old unpartitioned keys are deliberately NOT migrated. They may hold a
/// different account's credentials than the one now signed in, and there is no
/// way to tell whose they were — so adopting them is exactly the leak this
/// closes. They are left in place (untouched, unread) rather than deleted, so
/// a user who downgrades does not lose their settings.
pub(crate) fn settings_storage_key() -> String {
partitioned(STORAGE_KEY)
}
pub(crate) fn credential_storage_key() -> String {
partitioned(CREDENTIAL_STORAGE_KEY)
}
fn partitioned(base: &str) -> String {
format!("{base}::{}", crate::identity_epoch::current_subject())
}
#[path = "web_settings_legacy.rs"]
mod legacy;
#[path = "web_settings_storage.rs"]

34
crates/op-host-web/src/web_settings_acp_scrub_tests.rs
View file

@ -41,7 +41,7 @@ fn legacy_acp_in_separate_credentials_is_removed_without_losing_builtin_keys() {
);
assert!(state.editor_ui.agent_settings.acp_agents.is_empty());
assert_eq!(writes.len(), 1);
assert_eq!(writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(writes[0].0, super::credential_storage_key());
assert!(writes[0].1.contains("sk-must-survive-acp-removal"));
assert!(!writes[0].1.contains("acp_agents"));
assert!(!writes[0].1.contains("acp-command-secret"));
@ -91,7 +91,7 @@ fn separate_acp_scrub_does_not_touch_future_general_settings() {
assert!(load.unsupported_version);
assert!(load.initial_settings_fingerprint(&state).is_none());
assert_eq!(writes.len(), 1);
assert_eq!(writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(writes[0].0, super::credential_storage_key());
assert!(writes[0].1.contains("sk-must-survive-acp-removal"));
assert!(!writes[0].1.contains("acp_agents"));
assert!(!writes[0].1.contains("acp-command-secret"));
@ -143,8 +143,8 @@ fn acp_is_scrubbed_from_general_and_separate_snapshots_in_one_ordered_migration(
assert!(load.loaded);
assert!(!load.write_pending);
assert_eq!(writes.len(), 2);
assert_eq!(writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(writes[1].0, STORAGE_KEY);
assert_eq!(writes[0].0, super::credential_storage_key());
assert_eq!(writes[1].0, super::settings_storage_key());
for (_, json) in &writes {
assert!(!json.contains("acp_agents"));
assert!(!json.contains("general-acp-secret"));
@ -181,7 +181,7 @@ fn legacy_acp_configuration_is_removed_without_loading_or_migrating_it() {
assert!(!load.loaded);
assert!(!load.write_pending);
assert_eq!(writes.len(), 1);
assert_eq!(writes[0].0, STORAGE_KEY);
assert_eq!(writes[0].0, super::settings_storage_key());
assert!(!writes[0].1.contains("acp_agents"));
assert!(!writes[0].1.contains("connected"));
assert!(!writes[0].1.contains("legacy-command-secret"));
@ -214,7 +214,7 @@ fn failed_supported_acp_only_scrub_retries_without_authoritative_empty_credentia
assert!(!load.unsupported_version);
assert!(load.write_pending);
assert_eq!(initial_writes.len(), 1);
assert_eq!(initial_writes[0].0, STORAGE_KEY);
assert_eq!(initial_writes[0].0, super::settings_storage_key());
let mut baseline = load.initial_fingerprint(&state);
let mut retry_writes = Vec::new();
@ -226,7 +226,7 @@ fn failed_supported_acp_only_scrub_retries_without_authoritative_empty_credentia
assert!(saved.is_none());
assert!(!credential_migration_pending(&baseline));
assert_eq!(retry_writes.len(), 1);
assert_eq!(retry_writes[0].0, STORAGE_KEY);
assert_eq!(retry_writes[0].0, super::settings_storage_key());
assert!(!retry_writes[0].1.contains("acp_agents"));
assert!(!retry_writes[0].1.contains("connected"));
assert!(!retry_writes[0].1.contains("supported-acp-secret"));
@ -247,7 +247,7 @@ fn failed_supported_acp_only_scrub_retries_without_authoritative_empty_credentia
.is_some()
);
assert_eq!(user_writes.len(), 1);
assert_eq!(user_writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(user_writes[0].0, super::credential_storage_key());
assert!(user_writes[0].1.contains("sk-user-edit"));
}
@ -274,7 +274,7 @@ fn failed_read_only_credential_scrub_retries_without_enabling_user_writes() {
assert!(load.unsupported_version);
assert!(load.write_pending);
assert_eq!(initial_writes.len(), 1);
assert_eq!(initial_writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(initial_writes[0].0, super::credential_storage_key());
let mut baseline = load.initial_fingerprint(&state);
let mut retry_writes = Vec::new();
@ -286,7 +286,7 @@ fn failed_read_only_credential_scrub_retries_without_enabling_user_writes() {
assert!(saved.is_none());
assert!(!credential_migration_pending(&baseline));
assert_eq!(retry_writes.len(), 1);
assert_eq!(retry_writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(retry_writes[0].0, super::credential_storage_key());
assert!(!retry_writes[0].1.contains("acp_agents"));
assert!(!retry_writes[0].1.contains("acp-secret"));
assert!(retry_writes[0].1.contains("future-secret-must-survive"));
@ -329,7 +329,7 @@ fn failed_read_only_general_scrub_retries_without_creating_credential_snapshot()
assert!(load.unsupported_version);
assert!(load.write_pending);
assert_eq!(initial_writes.len(), 1);
assert_eq!(initial_writes[0].0, STORAGE_KEY);
assert_eq!(initial_writes[0].0, super::settings_storage_key());
let mut baseline = load.initial_fingerprint(&state);
let mut retry_writes = Vec::new();
@ -341,7 +341,7 @@ fn failed_read_only_general_scrub_retries_without_creating_credential_snapshot()
assert!(saved.is_none());
assert!(!credential_migration_pending(&baseline));
assert_eq!(retry_writes.len(), 1);
assert_eq!(retry_writes[0].0, STORAGE_KEY);
assert_eq!(retry_writes[0].0, super::settings_storage_key());
assert!(!retry_writes[0].1.contains("acp_agents"));
assert!(!retry_writes[0].1.contains("connected"));
assert!(!retry_writes[0].1.contains("general-acp-secret"));
@ -380,21 +380,21 @@ fn partial_read_only_scrub_retry_clears_only_the_successful_write() {
assert!(load.write_pending);
assert_eq!(initial_writes.len(), 1);
assert_eq!(initial_writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(initial_writes[0].0, super::credential_storage_key());
let mut baseline = load.initial_fingerprint(&state);
let mut first_retry = Vec::new();
assert!(
save_credentials_if_changed_with(&state, &mut baseline, |key, json| {
first_retry.push((key.to_string(), json.to_string()));
key == CREDENTIAL_STORAGE_KEY
*key == super::credential_storage_key()
})
.is_none()
);
assert!(credential_migration_pending(&baseline));
assert_eq!(first_retry.len(), 2);
assert_eq!(first_retry[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(first_retry[1].0, STORAGE_KEY);
assert_eq!(first_retry[0].0, super::credential_storage_key());
assert_eq!(first_retry[1].0, super::settings_storage_key());
assert!(first_retry[0].1.contains("future-credential-secret"));
assert!(!first_retry[0].1.contains("credential-acp-secret"));
@ -408,7 +408,7 @@ fn partial_read_only_scrub_retry_clears_only_the_successful_write() {
);
assert!(!credential_migration_pending(&baseline));
assert_eq!(second_retry.len(), 1);
assert_eq!(second_retry[0].0, STORAGE_KEY);
assert_eq!(second_retry[0].0, super::settings_storage_key());
assert!(second_retry[0].1.contains("future-general-secret"));
assert!(!second_retry[0].1.contains("general-acp-secret"));
}

4
crates/op-host-web/src/web_settings_lossless_tests.rs
View file

@ -238,7 +238,7 @@ fn acp_scrub_preserves_unknown_general_fields_and_leaves_snapshot_read_only() {
assert!(!load.loaded);
assert!(load.unsupported_version);
assert_eq!(writes.len(), 1);
assert_eq!(writes[0].0, STORAGE_KEY);
assert_eq!(writes[0].0, super::settings_storage_key());
assert!(writes[0].1.contains("future_setting"));
assert!(writes[0].1.contains("future-secret"));
assert!(!writes[0].1.contains("acp_agents"));
@ -266,7 +266,7 @@ fn acp_scrub_preserves_unknown_credential_fields_and_disables_future_writes() {
assert!(!load.loaded);
assert!(load.unsupported_version);
assert_eq!(writes.len(), 1);
assert_eq!(writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(writes[0].0, super::credential_storage_key());
assert!(writes[0].1.contains("future_credentials"));
assert!(writes[0].1.contains("future-credential-secret"));
assert!(!writes[0].1.contains("acp_agents"));

32
crates/op-host-web/src/web_settings_storage.rs
View file

@ -35,8 +35,8 @@ impl CredentialLoad {
}
pub(crate) fn load_into(state: &mut EditorState) -> CredentialLoad {
let settings_raw = storage_get(STORAGE_KEY);
let credential_raw = storage_get(CREDENTIAL_STORAGE_KEY);
let settings_raw = storage_get(&super::settings_storage_key());
let credential_raw = storage_get(&super::credential_storage_key());
let load = load_into_with(
state,
settings_raw.as_deref(),
@ -68,7 +68,7 @@ where
let sanitized = stored.sanitized_settings_json;
let settings_saved = sanitized
.as_deref()
.is_none_or(|json| persist(STORAGE_KEY, json));
.is_none_or(|json| persist(&super::settings_storage_key(), json));
CredentialLoad {
loaded: true,
write_pending: !settings_saved,
@ -85,11 +85,11 @@ where
let credential = stored.sanitized_credential_json.or(canonical_credential);
let credential_saved = credential
.as_deref()
.is_some_and(|json| persist(CREDENTIAL_STORAGE_KEY, json));
.is_some_and(|json| persist(&super::credential_storage_key(), json));
let settings_saved = credential_saved
&& sanitized
.as_deref()
.is_none_or(|json| persist(STORAGE_KEY, json));
.is_none_or(|json| persist(&super::settings_storage_key(), json));
let write_pending = !credential_saved || !settings_saved;
CredentialLoad {
loaded: true,
@ -106,7 +106,7 @@ where
let sanitized = stored.sanitized_settings_json;
let settings_saved = sanitized
.as_deref()
.is_none_or(|json| persist(STORAGE_KEY, json));
.is_none_or(|json| persist(&super::settings_storage_key(), json));
return CredentialLoad {
loaded: false,
write_pending: !settings_saved,
@ -121,11 +121,11 @@ where
let credential = credentials_json(state);
let credential_saved = credential
.as_deref()
.is_some_and(|json| persist(CREDENTIAL_STORAGE_KEY, json));
.is_some_and(|json| persist(&super::credential_storage_key(), json));
let settings_saved = credential_saved
&& sanitized
.as_deref()
.is_none_or(|json| persist(STORAGE_KEY, json));
.is_none_or(|json| persist(&super::settings_storage_key(), json));
let write_pending = !credential_saved || !settings_saved;
CredentialLoad {
// Both legacy credentials and a healed invalid separate
@ -146,12 +146,12 @@ where
let credential = stored.sanitized_credential_json;
let credential_saved = credential
.as_deref()
.is_none_or(|json| persist(CREDENTIAL_STORAGE_KEY, json));
.is_none_or(|json| persist(&super::credential_storage_key(), json));
let sanitized = stored.sanitized_settings_json;
let settings_saved = credential_saved
&& sanitized
.as_deref()
.is_none_or(|json| persist(STORAGE_KEY, json));
.is_none_or(|json| persist(&super::settings_storage_key(), json));
CredentialLoad {
loaded: false,
write_pending: !credential_saved || !settings_saved,
@ -166,7 +166,7 @@ where
let sanitized = stored.sanitized_settings_json;
let settings_saved = sanitized
.as_deref()
.is_none_or(|json| persist(STORAGE_KEY, json));
.is_none_or(|json| persist(&super::settings_storage_key(), json));
CredentialLoad {
loaded: false,
write_pending: !settings_saved,
@ -306,7 +306,9 @@ fn clear_local_credentials(state: &mut EditorState) {
}
pub(crate) fn save_if_changed(state: &EditorState, before: &mut Fingerprint) -> bool {
let saved = save_if_changed_with(state, before, |json| storage_set_checked(STORAGE_KEY, json));
let saved = save_if_changed_with(state, before, |json| {
storage_set_checked(&super::settings_storage_key(), json)
});
if saved {
clear_storage_failure();
} else if fingerprint(state) != *before {
@ -376,7 +378,7 @@ where
return None;
}
let json = credentials_json(state)?;
if !persist(CREDENTIAL_STORAGE_KEY, &json) {
if !persist(&super::credential_storage_key(), &json) {
return None;
}
*before = next;
@ -388,13 +390,13 @@ where
F: FnMut(&str, &str) -> bool,
{
if let Some(json) = before.pending_credential_json.clone() {
if !persist(CREDENTIAL_STORAGE_KEY, &json) {
if !persist(&super::credential_storage_key(), &json) {
return false;
}
before.pending_credential_json = None;
}
if let Some(json) = before.pending_settings_json.clone() {
if !persist(STORAGE_KEY, &json) {
if !persist(&super::settings_storage_key(), &json) {
return false;
}
before.pending_settings_json = None;

12
crates/op-host-web/src/web_settings_tests.rs
View file

@ -82,7 +82,7 @@ fn legacy_mcp_flags_drop_gemini_without_shifting_google_antigravity() {
);
let rewritten = writes
.iter()
.find_map(|(key, json)| (key == STORAGE_KEY).then_some(json))
.find_map(|(key, json)| (*key == super::settings_storage_key()).then_some(json))
.expect("legacy positional settings should be rewritten");
let value: serde_json::Value = serde_json::from_str(rewritten).expect("rewritten settings");
assert_eq!(
@ -380,7 +380,7 @@ fn future_credential_snapshot_remains_read_only_after_a_provider_edit() {
);
let mut writes = 0;
let saved = save_credentials_if_changed_with(&state, &mut baseline, |key, _| {
if key == CREDENTIAL_STORAGE_KEY {
if *key == super::credential_storage_key() {
writes += 1;
}
true
@ -487,7 +487,7 @@ fn corrupt_separate_snapshot_queues_its_persisted_empty_replacement_for_server_s
assert!(!load.write_pending);
let credential_write = writes
.iter()
.find(|(key, _)| key == CREDENTIAL_STORAGE_KEY)
.find(|(key, _)| *key == super::credential_storage_key())
.expect("the corrupt credential snapshot is replaced");
let value: serde_json::Value =
serde_json::from_str(&credential_write.1).expect("replacement is valid JSON");
@ -680,7 +680,7 @@ fn failed_legacy_credential_write_stays_loaded_and_retries_before_sanitizing() {
assert!(load.loaded);
assert!(load.write_pending);
assert_eq!(first_writes.len(), 1);
assert_eq!(first_writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(first_writes[0].0, super::credential_storage_key());
assert!(first_writes[0].1.contains("sk-retry-migration"));
let mut baseline = load.initial_fingerprint(&state);
@ -695,8 +695,8 @@ fn failed_legacy_credential_write_stays_loaded_and_retries_before_sanitizing() {
// sync. Retrying local persistence must not emit a second sync payload.
assert!(saved.is_none());
assert_eq!(retry_writes.len(), 2);
assert_eq!(retry_writes[0].0, CREDENTIAL_STORAGE_KEY);
assert_eq!(retry_writes[1].0, STORAGE_KEY);
assert_eq!(retry_writes[0].0, super::credential_storage_key());
assert_eq!(retry_writes[1].0, super::settings_storage_key());
assert!(!retry_writes[1].1.contains("sk-retry-migration"));
assert_eq!(baseline, credential_fingerprint(&state));
}