oficina / docs Public

core: fix Doc-construction ruling and enforce the entropy ban with clippy

Doc::with_client_id was the wrong safe alternative — it still draws a
random GUID via Options::with_client_id -> uuid_v4() one layer down.
The only construction that supplies both the client id and GUID itself
is Doc::with_options(Options::with_guid_and_client_id(guid, client_id));
wire_compat.rs now demonstrates that instead of the banned call.

Corrected two overstated claims in the doc comment: an entropy request
isn't "a bug we want to hear about" (fastrand::Rng::new() masks our Err
with a constant fallback seed rather than propagating it), and a fixed
seed doesn't make Observer::subscribe() ids collide within a process
(Rng::new() forks and advances the generator, so successive ids differ).
The real hazard is cross-process determinism: the same fixed seed on
every process means the same sequence of "random" ids/GUIDs everywhere,
invisible to any test that only runs one process.

Added clippy.toml (disallowed-methods banning yrs::Doc::new,
yrs::Doc::with_client_id, yrs::uuid_v4) and wired cargo clippy into
scripts/build-wasm.sh ahead of the build, since a doc comment alone has
been wrong three times now. Proved it bites with a temporary Doc::new()
call, reverted.

Also: getrandom moved to a wasm32-scoped target dependency (it's unused
on native builds), and any_to_json panics on Any::Undefined/Any::Buffer
instead of silently folding both into Value::Null.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011F5CivVm4CmL9ByfSSAaXA

Paul Campbell pushed by paul@keymail.dev 1 month ago 512c469af6b6216c2a560fb0e3e84fb70f6f9b67 parent 0cb352e

5 files changed, +81 −36

clippy.toml +5 −0
core/Cargo.toml +6 −0
core/src/lib.rs +42 −32
core/tests/wire_compat.rs +22 −4
scripts/build-wasm.sh +6 −0

Intent

full ledger: core-wire-compat

diff --git a/clippy.toml b/clippy.toml

new file mode 100644

index 0000000..a4922e4

--- /dev/null

+++ b/clippy.toml

@@ -0,0 +1,5 @@

+disallowed-methods = [

+ { path = "yrs::Doc::new", reason = "draws a random client id; use Options::with_guid_and_client_id" },

+ { path = "yrs::Doc::with_client_id", reason = "draws a GUID via uuid_v4; use Options::with_guid_and_client_id" },

+ { path = "yrs::uuid_v4", reason = "constant seed on wasm means identical values across processes" },

diff --git a/core/Cargo.toml b/core/Cargo.toml

index ea84dbf..887bd79 100644

--- a/core/Cargo.toml

+++ b/core/Cargo.toml

@@ -8,6 +8,12 @@ crate-type = ["cdylib", "rlib"]

[dependencies]

yrs = "=0.23.5"

+# Only used by the __getrandom_v03_custom hook in src/lib.rs, which is

+# itself cfg(target_arch = "wasm32") — the custom getrandom backend has no

+# role on native builds (tests, tooling), so keep the dependency scoped to

+# match instead of pulling it into every target unconditionally.

+[target.'cfg(target_arch = "wasm32")'.dependencies]

getrandom = "=0.4.3"

[profile.release]

diff --git a/core/src/lib.rs b/core/src/lib.rs

index 7afb0d1..21f4e70 100644

--- a/core/src/lib.rs

+++ b/core/src/lib.rs

@@ -1,48 +1,58 @@

//! The Docs core: Yjs CRDT and (later) text layout, compiled to wasm and

//! run both under wazero on the server and — for layout only — in the browser.

//!

-//! # Important: Client ID initialization

+//! # Important: Doc construction

//!

-//! Never call `yrs::Doc::new()` in this crate — always use `Doc::with_client_id()`.

-//! `Doc::new()` internally reaches for random entropy via fastrand, which pulls a JS

-//! import that the Go/wazero host cannot provide. Providing an explicit client ID

-//! avoids this and aligns with the design, which assigns client IDs deterministically.

+//! Never call `yrs::Doc::new()` *or* `yrs::Doc::with_client_id()` in this

+//! crate. Both draw entropy from `fastrand`, not just `Doc::new()`:

+//! `Doc::new()` picks a random client id *and* GUID; `Doc::with_client_id()`

+//! takes the client id explicitly but still draws a random GUID internally

+//! (`Options::with_client_id`, `yrs` `src/doc.rs:916`, calls `uuid_v4()`

+//! regardless). The only construction that supplies everything itself, and

+//! so is safe to use, is `Doc::with_options(Options::with_guid_and_client_id

+//! (guid, client_id))` (`doc.rs:928`) with both values we choose. This is

+//! enforced by `clippy.toml` (`disallowed-methods`), wired into

+//! `scripts/build-wasm.sh` — not just documented here, because this rule

+//! has been gotten wrong more than once already (see below).

//!

//! # Zero host imports and the custom getrandom backend

//!

-//! `yrs::Doc` unconditionally generates a random document GUID on construction

-//! (`Options::with_client_id` — used even by `Doc::with_client_id` — still calls

-//! `uuid_v4()` via `fastrand`; "client ID" and "GUID" are different fields).

-//! On `wasm32-unknown-unknown`, `yrs`'s pinned `fastrand` dependency requires

-//! the `js` feature, which makes the transitively-pulled `getrandom` crate

-//! default to a `wasm-bindgen`-based backend that imports host JS functions.

-//! Merely referencing `yrs::Doc` anywhere in this crate is enough for those

-//! `wasm-bindgen` exports (`__wbindgen_describe`, `__wbindgen_malloc`, ...)

-//! to end up linked into the final module, even along code paths that are

-//! never reached at runtime — which breaks the zero-imports contract with

-//! the Go/wazero host (see `core/tests/no_host_imports.rs`).

+//! On `wasm32-unknown-unknown`, `yrs`'s pinned `fastrand` dependency

+//! requires the `js` feature, which makes the transitively-pulled

+//! `getrandom` crate default to a `wasm-bindgen`-based backend that imports

+//! host JS functions. Merely referencing `yrs::Doc` anywhere in this crate

+//! is enough for those `wasm-bindgen` exports (`__wbindgen_describe`,

+//! `__wbindgen_malloc`, ...) to end up linked into the final module, even

+//! along code paths that are never reached at runtime — which breaks the

+//! zero-imports contract with the Go/wazero host (see

+//! `core/tests/no_host_imports.rs`).

//!

//! `docs-wt-core/.cargo/config.toml` selects getrandom's "custom" backend

//! for the `wasm32-unknown-unknown` target only (native builds, including

//! `cargo test`, are unaffected and keep the real OS entropy source). That

//! backend calls out to the `__getrandom_v03_custom` symbol defined below

-//! instead of any JS API.

+//! instead of any JS API. It refuses the request (`Err`) rather than

+//! fabricating bytes — but that refusal is weaker than it sounds, and the

+//! next paragraph is the actual reason construction must never draw a GUID

+//! or client id from the library, not "so a panic tells us":

//!

-//! This crate assigns every ID it hands to `yrs` deliberately —

-//! `Doc::with_client_id` takes an explicit client ID — so nothing here

-//! legitimately needs entropy. But `yrs` itself draws entropy in a couple

-//! of places that are unreachable from what this crate does *today* and

-//! therefore untested by it: `Observer::subscribe()` (`yrs` src/observer.rs,

-//! both the `sync` and non-`sync` impls) draws a subscription ID from

-//! `fastrand::Rng::new()` and uses it as an `Origin` — with a fabricated

-//! fixed value, every subscription would collide and cancelling one would

-//! cancel another; `uuid_v4()` (`yrs` src/lib.rs) draws a document GUID the

-//! same way — with a fabricated fixed value, every document would share a

-//! GUID. Both become reachable once live document sync (this project's

-//! later stages) actually subscribes to change events or constructs fresh

-//! documents. An entropy request from this crate is therefore a bug we want

-//! to hear about the moment it happens, not a value to fabricate — so this

-//! hook fails instead of returning fake bytes. Fail closed, not fake-succeed.

+//! `fastrand::Rng::new()` — what both `Observer::subscribe()`

+//! (`yrs` `src/observer.rs:128`/`:145`, subscription ids) and `uuid_v4()`

+//! (`yrs` `src/lib.rs:713`, document GUIDs) call — does not propagate our

+//! `Err`. It masks it: `vendor/fastrand-2.5.0/src/global_rng.rs:223` turns

+//! any `getrandom` failure into `None` and falls back to a constant seed.

+//! `Rng::new()` itself then *forks* the thread-local generator from that

+//! seed and advances it, so successive calls within one process still

+//! produce different values — subscription ids don't collide with each

+//! other, and this crate's own tests (which never construct more than one

+//! `Doc` per process) can't see anything wrong. The actual hazard is

+//! cross-process: the thread-local seed is the same fixed constant on

+//! every process, every time, so the *sequence* of "random" ids/GUIDs a

+//! process draws is identical across processes. Two independently-created

+//! documents that never draw a client id or GUID explicitly would collide.

+//! This crate avoids the whole problem by supplying both explicitly (see

+//! above) rather than relying on `fastrand` to fail loudly when it can't —

+//! it won't.

#[cfg(target_arch = "wasm32")]

#[no_mangle]

extern "Rust" fn __getrandom_v03_custom(

diff --git a/core/tests/wire_compat.rs b/core/tests/wire_compat.rs

index fac6bc2..7ae9471 100644

--- a/core/tests/wire_compat.rs

+++ b/core/tests/wire_compat.rs

@@ -4,7 +4,7 @@ use std::path::PathBuf;

use serde_json::{json, Value};

use yrs::types::text::YChange;

use yrs::types::Attrs;

-use yrs::{Any, Doc, Text, Transact};

+use yrs::{Any, Doc, Options, Text, Transact};

fn fixture(name: &str) -> (Vec<u8>, String, Value) {

let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"))

@@ -28,12 +28,22 @@ fn fixture(name: &str) -> (Vec<u8>, String, Value) {

fn any_to_json(any: &Any) -> Value {

match any {

Any::Null => Value::Null,

- Any::Undefined => Value::Null,

Any::Bool(b) => Value::Bool(*b),

Any::Number(n) => json!(n),

Any::BigInt(n) => json!(n),

Any::String(s) => Value::String(s.to_string()),

- Any::Buffer(_) => Value::Null,

+ // A Yjs delta's `attributes` map comes from JSON-compatible formatting

+ // marks (bold: true, and similar) — never `undefined` or a binary

+ // buffer. Folding either into `Value::Null` would let a future

+ // fixture with one of these silently compare equal to a bare

+ // `null` attribute value instead of failing loudly on a shape this

+ // test doesn't know how to compare.

+ Any::Undefined => unreachable!(

+ "Any::Undefined in a delta attribute — not representable in the fixture JSON"

+ ),

+ Any::Buffer(_) => unreachable!(

+ "Any::Buffer in a delta attribute — not representable in the fixture JSON"

+ ),

Any::Array(items) => Value::Array(items.iter().map(any_to_json).collect()),

Any::Map(map) => {

let mut entries: Vec<(&String, &Any)> = map.iter().collect();

@@ -84,7 +94,15 @@ fn delta_of(doc: &Doc) -> Value {

fn check(name: &str) {

let (update, expected_text, expected_delta) = fixture(name);

- let doc = Doc::with_client_id(9001); // never Doc::new() — see Global Constraints

+ // Never yrs::Doc::new() (draws a random client id) or

+ // yrs::Doc::with_client_id() (still draws a random GUID) — both reach

+ // fastrand entropy that this crate's custom getrandom hook refuses to

+ // fabricate. Supply both the client id and the GUID explicitly via

+ // Options::with_guid_and_client_id instead; this is the construction

+ // docs-core itself must use everywhere (see core/src/lib.rs and

+ // clippy.toml, which bans the other two at the source level).

+ let guid: yrs::Uuid = "wire-compat-test-guid".into();

+ let doc = Doc::with_options(Options::with_guid_and_client_id(guid, 9001));

docs_core::apply_update(&doc, &update).expect("apply failed");

assert_eq!(

docs_core::text_content(&doc),

diff --git a/scripts/build-wasm.sh b/scripts/build-wasm.sh

index df8d4eb..d2cf2e2 100755

--- a/scripts/build-wasm.sh

+++ b/scripts/build-wasm.sh

@@ -3,6 +3,12 @@

# provides no JS glue, so the module must be plain wasm32 with a C ABI.

set -euo pipefail

cd "$(dirname "$0")/.."

+# Lint before building: disallowed-methods (clippy.toml) enforces the

+# Doc-construction rule documented in core/src/lib.rs — Doc::new() and

+# Doc::with_client_id() both draw entropy this build has no safe source

+# for. A doc comment alone has been wrong, or gotten around, more than

+# once; this is the actual enforcement.

+cargo clippy --manifest-path core/Cargo.toml --all-targets -- -D warnings -D clippy::disallowed_methods

cargo build --release --manifest-path core/Cargo.toml --target wasm32-unknown-unknown --locked

out="core/target/wasm32-unknown-unknown/release/docs_core.wasm"

[ -f "$out" ] || { echo "build-wasm: no artifact at $out" >&2; exit 1; }