Architecture Review — 2026-10-27
- Status: the first architecture review (plan/06, plan/12 §7), run after the 19 work packages of Phase 0 that are Done. Its findings are recommendations: the orchestrator turns them into follow-up tasks or carries them into the Phase-0 gate (plan/12 §6), and the items marked owner are matters that only the owner can settle (plan/12).
- What was reviewed: the
mainbranches of 2026-10-27: bayan-core atbc903b2, bayan-desktop atb9b217a, bayan-web at01c5978and bayan-server at3d6e9ea, against this repository at0f1eb21. Every link into code points to those commits, so it stays valid when the code changes. - Method: a read-only review against AGENTS.md, plan/03, the Accepted ADRs and the specs. Six helper agents read the four repositories in parallel; the reviewing session then checked every finding below against the code itself and reproduced the two memory findings (H3 and M3) with a small measurement program outside the repositories. Nothing in the code repositories was changed.
Summary for the owner
The foundations are in good shape where it matters most. The crates of bayan-core respect their layers, and nothing in the product depends on a spike. Unsafe Rust exists only in the C binding, where every block is justified. The determinism rules (integer layout units, no hash-map order, no platform math) are followed and enforced by lints. Neither app lays out, measures or hit-tests text itself; both leave that to the engine. The server never sees or logs document content. All 38 CI workflows in the five repositories follow the security conventions, apart from the hygiene gaps listed in L3, and the files that must be identical in every repository are identical. The section What is in good shape lists more.
The problems sit at the edges between the pieces and in the safety nets. Seven are rated high:
- Some safety nets exist only on paper. GitHub’s branch rules do not yet require the workflow lint or the CodeQL scans that the settings baseline lists (H1). The engine’s artifact tests can fail without blocking a merge: they are the only run of Miri, the checker for unsafe code, and the only tests of the web worker host (H1). Fuzzing, which throws millions of malformed inputs at a parser to find crashes, never runs automatically (H2).
- Two parsers can be pushed far past their memory limits. A crafted 4 MB legacy Office file can make the engine run out of memory and stop (H3), and the XML reader can use several times its documented maximum (M3). No parser can be stopped or held to a time limit yet, which ADR-0006 requires (H6). Nothing reaches these parsers from a user’s document today, so this is the right moment to fix them.
- The contract between the engine and the two apps is incomplete. Each app has filled the holes in its own way, and the two already behave differently for keys, shortcuts and crash recovery (H5, M6). One hole will make the desktop app show outdated page images once the engine lays out pages in the background (H5).
- The server’s copy of the web app’s security headers is weaker than the web app’s own (H4). On the deployment the ADRs make the default, it drops the rule that only one Trusted Types policy may exist.
- Several things the ADRs require of Phase 1 are missing from the Phase-1 drafts (H7, M1). These are the core MLS crate that the server’s validator must come from, the conditions you set when you kept Loro, and the protocol additions the app spikes asked for. The Phase-0 gate is the place to add them.
Five items are yours alone:
- report H3’s cfb case privately to cfb’s maintainer (H3); the orchestrator holds the reproduction;
- run the GitHub settings audit and apply it (H1, issue #15);
- confirm that Loro’s crash reports went out (M12);
- decide the license of the shared supply-chain checker (L2);
- schedule the deferred screen-reader and input-method tests before the accessibility work packages start (M9).
Everything else is work for agent sessions, mostly small.
How to read this report
- Severity. High: breaks an Accepted ADR or a spec, or is a security weakness with a realistic path, and will spread into Phase-1 work if not fixed now. Medium: drift or a gap with limited reach, or missing tests for important behavior. Low: hygiene and documentation drift. Nothing found is exploitable through a released product today, because nothing has been released and no user document reaches the parsers yet, so no finding is rated critical.
- Size. S: one small pull request, under a day. M: a few days, one work package. L: several work packages.
- Known. Whether the problem was already recorded somewhere (an ADR, a report, a code comment). A known problem is listed when no work package or follow-up owns it yet.
Findings at a glance
| ID | Severity | Finding | Repositories | Size | Who acts |
|---|---|---|---|---|---|
| H1 | High | The merge gates do not enforce what the ADRs and the settings baseline require | all five | S | owner, then docs and bayan-core |
| H2 | High | Fuzzing never runs in CI, and four kinds of untrusted input have no fuzz target | bayan-core | M | bayan-core follow-up |
| H3 | High | A crafted compound file makes bayan-opc use about a thousand times its size in memory | bayan-core | S–M | bayan-core follow-up; owner reports upstream |
| H4 | High | The server serves the web app with a weaker copy of its security headers | bayan-server, bayan-web | S, then M | bayan-server follow-up; WEB-103 and SRV-105 |
| H5 | High | The engine protocol leaves behavior to the shells, and they already decide differently | docs, all code | M | protocol (planning) session; CORE-125 |
| H6 | High | No parser can be stopped or held to a time budget | docs, bayan-core | M | ADR-0006 amendment, then bayan-core |
| H7 | High | Phase-1 server work cannot take MLS and protocol code from bayan-core, as the ADRs require | docs, bayan-server, bayan-core | L | Phase-0 gate |
| M1 | Medium | Conditions from ADR amendments and the CORE-004 report are missing from the Phase-1 drafts | docs | S | Phase-0 gate |
| M2 | Medium | The CRDT adapter does not guard every path that untrusted bytes take | bayan-core, docs | S, plus a decision | CORE-101; ADR-0016 or SRV-104 |
| M3 | Medium | bayan-xml exceeds its documented memory bound on two crafted inputs | bayan-core | S | bayan-core follow-up |
| M4 | Medium | Two interim parsers outlived their replacements | bayan-core | M | bayan-core follow-up, before LAB-101 |
| M5 | Medium | The outside-world rule is enforced only by review, and the engine already reads the environment | bayan-core | S | bayan-core follow-up |
| M6 | Medium | Neither shell recovers from an engine panic as the protocol says | bayan-desktop, bayan-web | S | shell follow-ups |
| M7 | Medium | The desktop’s tests and sanitizers miss the real engine boundary | bayan-desktop | M | bayan-desktop follow-up |
| M8 | Medium | Two lockfiles that CI compiles are outside the automatic 24-hour check | bayan-core, bayan-server | S | one change in both |
| M9 | Medium | No automated accessibility checks, and the manual validation passes are still open | bayan-web, bayan-desktop | S | bayan-web follow-up; owner |
| M10 | Medium | PostgreSQL: the plan lets account data arrive before TLS, and the TLS defaults are not fixed | docs, bayan-server | S | Phase-0 gate; SRV-102 |
| M11 | Medium | The server’s two migration copies are kept in step by a comment | bayan-server | S | bayan-server follow-up |
| M12 | Medium | Nothing records that Loro’s crash findings reached its maintainers | docs | S | owner |
| L1 | Low | Loro is imported outside bayan-crdt, in tests | bayan-core | S | bayan-core |
| L2 | Low | The shared supply-chain checker is GPL in one repository and AGPL in the other | bayan-core, bayan-server | S | owner |
| L3 | Low | Workflow and setup hygiene | all code | S | with the next CI change |
| L4 | Low | Latent weaknesses in the shells | bayan-desktop, bayan-web | S | DESK-101, WEB-101 |
| L5 | Low | Server hardening before the Phase-1 endpoints | bayan-server | S | SRV-101 to SRV-103 |
| L6 | Low | Engine boundary details | bayan-core | S | CORE-125 |
| L7 | Low | Parser details | bayan-core | S | bayan-core |
| L8 | Low | Leftovers of finished spikes | bayan-core | S | bayan-core |
| L9 | Low | Qt’s archive hashes come from Qt’s server at install time | bayan-desktop | S | monthly dependency session |
| L10 | Low | Nothing checks that the shared CI files stay identical | docs | S | docs |
| L11 | Low | Documentation drift | docs, all code | S | docs and each repository |
High
H1. The merge gates do not enforce what the ADRs and the settings baseline require
What we found.
- The live rules lag the baseline. A ruleset is GitHub’s protection of a branch; its required checks are the CI jobs that must pass before anything can be merged. The ruleset that protects
mainin this repository was created on 2026-10-06 and last changed on 2026-10-09 (read from GitHub’s public API on 2026-10-27). It requires five checks:scripts/verify.sh,DCO,REUSE lint,No update botsandpip-audit. The settings baseline, scripts/github-settings.py, also listsWorkflow lint,CodeQL (actions),CodeQL (python)andCodeQL (javascript-typescript). X-002 added those on 2026-10-10, and they were never applied. The code repositories’ rulesets could not be read from this session; they were probably applied at the same time and lag in the same way. - The baseline leaves out the artifact tests. bayan-core’s
artifacts.ymlruns on every pull request, but none of its jobs is required, and it does not run in the merge queue (nomerge_grouptrigger). Its jobs are the only CI run of three things. The first is bayan-ffi’s tests under Miri, an interpreter that detects undefined behavior inunsafecode (Miri (bayan-ffi)). The second is the tests of the worker hostbayan-worker.js, the web boundary that receives the page’s messages, together with the check that engine panics print nothing. The third is the check that every platform renders the same tile (Same tile everywhere). Each can fail without blocking a merge;cargo xtask verifyruns none of them. - The owner’s records disagree with GitHub. Issue #15 (“Apply the GitHub settings baseline …”) is open and items 3 to 6a of the owner checklist are unticked, although rulesets exist.
Why it matters. AGENTS.md §4 says the workflow lint and CodeQL “must pass”. ADR-0006 decision 2 requires Miri coverage of unsafe code. The threat model counts T4 (hostile messages at the boundary) as in place, and T20 counts the engine’s silence as tested. From today the orchestrator merges approved work without the owner. Its procedure checks every check, not only the required ones (plan/12 §5). The ruleset is the backstop when a result is missed, and GitHub lost two job results on docs#49 that same day.
Known: partly. Issue #15 records the owner’s choice of 2026-10-05 to apply the baseline only once X-001 to X-003 were Done, which they now are. Nothing records that the live rules lag the baseline, or that the baseline leaves out the artifact tests.
Recommended fix (S).
- Owner: run
python3 scripts/github-settings.py audit, thenapply, for all five repositories, as developer/github-settings.md describes. Then tick checklist items 3 to 6a and update or close issue #15. - A docs pull request: add
Same tile everywhereandMiri (bayan-ffi)to bayan-core’s required checks in the baseline.Same tile everywherealready fails when any driver job failed or was skipped. The owner then applies the change. - A bayan-core pull request: add
merge_group: types: [checks_requested]toartifacts.yml. The alternative is to move the Node.js driver and Miri intocargo xtask verify.
H2. Fuzzing never runs in CI, and four kinds of untrusted input have no fuzz target
What we found.
Fuzzing feeds a parser millions of random and mutated inputs to find crashes and hangs. bayan-core has eight fuzz targets, all for bayan-opc and bayan-xml (fuzz/Cargo.toml). It runs them only by hand, in the session that changes a parser, which records in its pull request how long they ran (fuzz/README.md). No workflow in any repository builds or runs them, so a change to a parser’s interface can even break its target unnoticed.
Four kinds of input that come from outside have no target at all:
- Engine protocol messages, tile requests and the configuration. The engine’s hostile-input test calls itself “a deterministic, bounded stand-in for fuzzing until the fuzzing workspace exists” (hostile_input.rs:3), and that workspace exists now.
- Recordings that
diag.replayreads, which the spec treats as untrusted (§10). - CRDT updates from other people. The only fuzzer for them is the CORE-004 spike’s own mutator, written before the owner accepted libFuzzer’s license (fuzz.rs:3).
- The Fidelity Lab’s own ZIP, package and XML readers (M4).
The fuzz README’s first line says protocol messages have a target. They do not.
Why it matters.
- ADR-0025 decision 4: “short fuzz runs on every pull request touching it, continuous fuzzing on a schedule”.
- ADR-0008 decision 4: the CRDT import path “is fuzzed”.
- plan/07 §2 names “fuzz smoke runs” in bayan-core’s pull-request gate and “long fuzzing” nightly. Its testing table lists engine protocol messages among the parsers to fuzz.
- Phase 1 adds about a dozen parsers (CORE-102 to CORE-106, CORE-110, CORE-122, CORE-123). CORE-102’s exit evidence already asks for a “fuzz target running in CI”, but no work package builds the CI job.
- H3 shows what a parser can still hide after CORE-005’s 5.97 CPU-hours of fuzzing by hand.
Known: partly. ADR-0017’s amendment of 2026-10-08 calls CI fuzzing “a follow-up”; nothing schedules it.
Recommended fix (M, one bayan-core work package).
- A
fuzz.ymlworkflow. It builds every target with the pinned nightly toolchain and cargo-fuzz 0.13.2, checked against a pinned SHA-256. On every pull request and in the merge queue, it runs each target for a minute or two from seed corpora committed underfuzz/corpus/and made from the existing test fixtures. Every night, it runs each target for longer. Use GitHub’s own runners: ClusterFuzzLite, which plan/07 mentions, would need the owner’s approval as a third-party action. - Crash inputs stay private. A crash is handled as a security report (SECURITY.md), not uploaded as a public workflow artifact.
- New targets: engine message handling after a handshake,
diag.replay,bayan_render_tile’s request and the configuration, and acrdt_importtarget that reuses the spike’s mutations. - Clean-up: correct the README’s first line, and make the pull-request job required once it is stable (H1).
H3. A crafted compound file makes bayan-opc use about a thousand times its size in memory
What we found. OLE compound files (Microsoft’s Compound File Binary format) contain three things BayanDocs will open: legacy .doc files, the VBA projects inside .docm files, and password-encrypted documents.
- Nothing limits cfb’s own work.
CompoundFile::openhands the whole input to the cfb crate before any of bayan-opc’s limits apply. - The cfb case. A crafted compound file makes cfb 0.15.0 allocate about a thousand times its size before it refuses the file as malformed. Reproduced in this review on the reviewed commit, in a release build, a 266 KB file reached a peak of 265 MB and a 1 MB file 1.05 GB, taking 3 s. A 4 MiB file asks for 4 GiB. On the desktop an allocation failure that large ends the process with an abort, which the C interface cannot catch; in the browser it stops the engine’s WebAssembly instance like a panic, and the worker host restarts the engine, losing the open document. The case affects every program that reads compound files with cfb 0.15.0, so how such a file is built is withheld until the owner has reported it privately to cfb’s maintainer; the brief of the follow-up that fixes it will carry the details.
- The nesting case. Separately, opening keeps the full path of every entry, twice (compound_file.rs:113, :127). Storages nested inside each other therefore cost memory and time that grow with the square of their depth. 2,000 nested storages in a 266 KB file took 229 MB and were accepted, well within the default limit of 65,536 entries.
- Promise and tests. The crate documents a limit for compound files (lib.rs:59) that bounds neither case, and neither file is in its tests or fuzz corpus.
Why it matters. ADR-0006 decision 5 says every parser enforces limits on size, depth and count. The threat model counts T1 and T3 as “in place for bayan-opc”. Nothing reaches this code today, but it is the planned entry point for .doc import, VBA projects and encrypted packages.
Known: no. The owner’s acceptance of cfb on 2026-10-10 concerned only the unsafe code of its uuid dependency.
Recommended fix (S–M, bayan-core).
- Check the file’s structure before calling cfb, so that what cfb allocates is bounded by the file’s size; the follow-up’s brief gives the details.
- Add a size limit and a depth limit for compound files to
Limits. - Keep each entry’s parent and name instead of its full path.
- Add both crafted files as regression tests and as fuzz seeds; the cfb one only once the owner says the upstream report allows it to be published.
- Owner: report the cfb case privately to cfb’s maintainer, as the owner did for Loro’s findings; the orchestrator holds the reproduction.
H4. The server serves the web app with a weaker copy of its security headers
What we found.
The browser enforces a page’s protection through HTTP headers, chiefly the Content Security Policy (CSP), which says what the page may load and run. Its Trusted Types rules allow script addresses only through named, reviewed policies. The web app’s headers exist in three places:
- bayan-web defines them in
config/security-headers.tsand repeats them indeploy/nginx/security-headers.conf. A unit test keeps those two equal. - bayan-server serves the built web app from
BAYAN_WEB_DIR(http/mod.rs:45), as ADR-0015 decision 2 makes the default deployment. It has a third copy (headers.rs:11), written in SRV-001 on 2026-10-05 and never changed since. It was never in step: WEB-001 already had stricter values the day before.
The server’s CSP has no trusted-types directive and no frame-src 'none', and it has form-action 'self' instead of 'none'. Its Permissions-Policy differs too. Without a trusted-types directive, browsers let the page’s scripts create any number of Trusted Types policies, including a permissive default policy. The server’s test compares its responses with the server’s own constant (tests/http.rs:14), so no test can see the drift.
The way the app is deployed is unsettled as well:
- WEB-103 plans a container image of its own, and a public demo on GitHub Pages, which cannot send custom headers at all: no
Integrity-Policy, noframe-ancestorsand no cross-origin isolation. - The server’s README says that
docker rungives a complete deployment. The image contains no web app, though: operators mount a build themselves, and no work package puts it in the image.
Why it matters.
- ADR-0014’s amendment of 2026-10-12, design detail 1: the CSP “is WEB-001’s with one change:
trusted-types bayan-script-url”, and “any further policy needs an amendment of this ADR”. - ADR-0002 decision 6: no duplication across repositories.
- Threat model F16.3. The one-policy rule is part of what keeps an injected script away from the keys the web app will hold (T11).
Known: no. The threat model’s endpoint requirements assume the server sends ADR-0014’s headers.
Recommended fix.
- Now (S, bayan-server): make the server’s headers equal to bayan-web’s current set, and test them against values copied from a named bayan-web commit.
- Next (M, WEB-103 with SRV-105 or X-101): make the web app’s build the single source. The build writes its headers and cache rules into the built app, and the server applies exactly those to the files under
BAYAN_WEB_DIRand refuses to start without them. The nginx file is generated from the same source. - Deployment decisions: settle which deployments exist and put the web app into the server’s image, or correct the README. Either drop the GitHub Pages demo or record it in an ADR-0014 amendment as a demo without enforced integrity.
H5. The engine protocol leaves behavior to the shells, and they already decide differently
What we found.
The two shell spikes listed what engine protocol v0 lacks: ADR-0013’s validation result, the DESK-002 report and ADR-0014’s amendment. The DESK-002 report recommended “one contract-first pull request before DESK-101 and DESK-103 need them, starting with the first, on which tile freshness depends” (DESK-002-report.md:146). No such pull request exists. The spec is unchanged since CORE-007, and the CORE-125 draft (protocol v1) lists none of the gaps. Meanwhile the shells filled them in different ways:
| Gap | bayan-desktop | bayan-web | Consequence |
|---|---|---|---|
Which request caused a render.invalidate | Assumes every invalidation belongs to the request whose reply arrived last, an order the spec promises only for view.set (documentsession.cpp:50, :164; tilecache.cpp:125) | Counts invalidations per tile (compositor.ts:127) | Once the engine invalidates on its own, as background layout will (plan/03 §6.1), the desktop keeps a tile rendered between the last reply and the change: outdated content on screen, with no error |
| Which key presses reach the engine | Nearly all; printable keys as both input.key and input.text (keymapping.h:10), but zoom (Ctrl+=, Ctrl+−, Ctrl+0), PageUp, PageDown and Ctrl+Home/End stay in the shell (documentcanvas.cpp:733) | Only a fixed list of editing keys, including PageUp, PageDown, Home and End, plus the manifest’s shortcuts, which the shell parses itself (keys.ts:9, :35, :82) | The engine receives different input for the same keystrokes; the desktop’s zoom keys hide Word shortcuts (Ctrl+= is subscript) |
| The platform, for matching “Ctrl” against Command | Names the operating system (documentsession.cpp:37) | Says "web" and maps Ctrl to Command itself (document-controller.ts:110, :239) | The owner decided that the engine does this matching (ADR-0014 amendment, for CORE-125); the CORE-125 draft does not say so |
| Replacing text near the caret (input methods, autocorrect, screen readers) | Sends one Backspace or Delete per character (documentcanvas.cpp:345) | Inserts the new text at the caret and leaves the replaced range in place (hidden-element-bridge.ts:117) | The desktop will delete the wrong amount once the engine deletes whole grapheme clusters; on the web the old text stays next to the new |
The spikes also found other gaps, which neither shell can fill:
- hit-testing (finding the character under a point);
- moving the caret to an offset;
- selection and line ranges in the accessibility tree, and the composition’s range;
- clipboard bytes;
- invalidation narrower than every page on
view.set; - the worker host accepting the engine module’s hash (bayan-web issue 8), which threat model §13.3 item 4 requires before collaboration runs in browsers.
This review adds one more. Accessibility nodes carry no language or direction, and the web mirror marks the whole document as English, left to right (index.html:2). Screen readers need both to choose a voice (ADR-0020 decision 2, “text with attributes”; ADR-0021 decisions 3 and 4).
Why it matters.
- The protocol’s first goal: “One contract for both shells, so behavior cannot drift” (spec §1).
- ADR-0002 decisions 2 (contract-first) and 6.
- plan/03 §3: “If two shells would need the same logic, it belongs in core”.
- DESK-101 and WEB-101 will build navigation, find and copy on exactly these paths.
Known: each gap is in a spike report. The divergence between the shells, the reach of the tile-freshness assumption (invalidations that no request caused) and the missing language are not recorded, and no brief carries any of it.
Recommended fix.
M: one contract-first pull request to specs/engine-protocol.md, from a planning session before the Phase-0 gate refines CORE-125, DESK-101 and WEB-101. It should cover:
- a render revision on
render.invalidateand on every tile (or tiles delivered in order with events, as on the web); - which key presses a shell forwards (proposal: all, with the shell deciding only which to keep from the browser or the operating system), with the manifest’s shortcuts as structured bindings per platform;
hello.shell.platformnaming the operating system;input.textwith a range to replace;- a hit-test query and a message that moves the caret;
- selection, line ranges, language and direction in accessibility nodes;
- clipboard bytes and narrower invalidation;
- the module hash in the worker host’s
init.
Then add these to CORE-125’s scope.
S, now: the desktop drops its Ctrl+=, Ctrl+− and Ctrl+0 bindings and forwards PageUp, PageDown, Home and End as the web does.
H6. No parser can be stopped or held to a time budget
What we found.
- The rule. ADR-0006 decision 5 requires every parser and layout pass to enforce limits “on size, depth, count and time”, and to be “cancellable”. plan/03 §12 repeats it.
- No time limit or cancellation in the parsers. bayan-opc and bayan-xml limit sizes, depths and counts, but neither has a time or work budget, nor any way to stop. Opening a package, reading a ZIP archive, writing a package (which decompresses every part to check it), opening a compound file and canonicalizing XML always run to the end. H3 showed a 3-second run that nothing could interrupt.
- No way to ask the engine to stop. The C interface has no such function and the protocol no such message. On the web, the main thread can only terminate the whole worker.
- A time limit has to be counted work. The engine may not read a clock (ADR-0012 decision 5), so a time limit has to be a deterministic budget of counted work.
Why it matters. It is a rule of an Accepted ADR. The Phase-1 importers (CORE-102 to CORE-107) and layout (CORE-113) will be built on these function signatures. Background layout (plan/03 §6.1) needs cancellation to stay responsive.
Known: no.
Recommended fix (M).
- Decide first, in an ADR-0006 amendment from a planning session, before CORE-102 is refined:
- a work budget counted in units of work (bytes decoded, events, nodes), which callers pass in;
- a cancellation flag that the host can set between messages: on desktop, for example, a tenth C function; on the web, a shared flag, which the web app’s cross-origin isolation already allows, or a documented restart of the worker;
- where the budget and the flag are checked;
- one
Limitsvalue that configures both crates.
- Then a bayan-core work package adds the budget and the flag to bayan-opc and bayan-xml.
H7. Phase-1 server work cannot take MLS and protocol code from bayan-core, as the ADRs require
What we found.
- Where the ADRs put this code. ADR-0016 decision 1 puts MLS in a core crate,
bayan-mls, “shared with the server for validation”. ADR-0002 decisions 4 and 6 have the server consume core crates as Git dependencies pinned to a tag and commit, so that logic needed by both is written once. - Where the Phase-1 drafts put it. No Phase-1 work package creates
bayan-mls. SRV-103 builds “public-state validation of commits and role enforcement” in the server, and SRV-104 creates bayan-core’sbayan-protocolcrate, “used by both core and server” (server drafts). - The server cannot depend on bayan-core today.
deny.tomlallows no Git source (deny.toml:69).- The lockfile check refuses every package from a Git repository (lockfile_age.rs:257).
- bayan-core has no tags.
- ADR-0017’s amendment of 2026-10-12 says the server’s Git dependencies on core need “a decision of its own”.
- The SRV-002 spike’s validator is not a starting point as it stands. It lives in bayan-server, under the AGPL, and:
- accepts every ciphersuite the build knows, the provisional post-quantum one included (validator.rs:106);
- refuses the external commits that ADR-0029 now needs for resyncs and recovery joins;
- takes randomness and time from the platform (threat model §13.3 item 2);
- uses a RustCrypto provider that brings
crypto-bigint0.5.5, with the timing advisory RUSTSEC-2026-0387. bayan-server’sdeny.tomlexcepts that advisory only because the spike is never built into the server (deny.toml:15).
Why it matters. Without a decision, the role policy and the commit validator get written in bayan-server in Phase 1, and again in bayan-mls in Phase 3. That is the duplication ADR-0002 forbids, and in the security-critical code of the project.
Known: partly. bayan-server’s AGENTS.md says that “the work package that needs a Git dependency teaches it first”, and the threat model §13.3 lists the spike’s platform calls. The missing core work package and the crypto-bigint conflict are not recorded.
Recommended fix (L, at the Phase-0 gate).
- A core work package for
bayan-mlsv0, ahead of SRV-103: the role policy, commit and key-package validation, the commit types of ADR-0029 and ADR-0030, randomness and time from the host, and no provisional suites by default. The alternative is an explicit decision that SRV-103 builds against a stand-in for it. - A decision on how the server pins core commits. Keeping the 24-hour minimum, as bayan-web does for its engine pin, needs no relaxation of ADR-0017. It needs a change to
check-lockfile-agein both repositories that accepts Git packages only from BayanDocs/bayan-core, at a commit onmainthat is at least 24 hours old. - A note in the
bayan-mlsbrief that its provider must not needcrypto-bigint0.5, or that the work waits forelliptic-curve0.14. - A “constraints changed since the spike” section in the spike’s report,
spikes/mls/REPORT.mdin bayan-server.
Medium
M1. Conditions from ADR amendments and the CORE-004 report are missing from the Phase-1 drafts
What we found.
- The conditions on Loro. When the owner kept Loro (ADR-0008, amendment of 2026-10-08), three conditions came with it:
- wait for Loro’s fix of the quadratic decoding (F1) before measuring loading again;
- the hosts isolate imports of untrusted updates and restart an engine that aborts;
- CORE-101 addresses the per-paragraph maps and the latency of applying updates and of undo.
- Other ADRs. ADR-0007’s validation result gives CORE-101 the preserved atoms. ADR-0012’s amendment asks the work package that first links
bayan-crdtinto the engine to check the engine module’s imports. - The CORE-004 report. Its follow-ups (§11) add, among others:
- What the drafts say. The CORE-101 draft gives only an objective and exit evidence. No Phase-1 draft, for core, desktop, web or server, mentions isolation, restart, the F1 fix or preserved atoms.
Why it matters. The conditions are the owner’s decision and binding. CORE-101 starts the critical path, and a brief written only from the draft would drop them.
Known: in the ADRs and the report; not in any brief.
Recommended fix (S, at the Phase-0 gate).
- Fold these items into CORE-101’s scope and acceptance criteria, with “depends on a Loro release with the F1 fix”.
- Give isolation and restart to the work package that first imports updates from other people, together with the shells’ restart policy (M6).
- Name the work package that first links
bayan-crdt, and give it the import check (M5).
M2. The CRDT adapter does not guard every path that untrusted bytes take
What we found.
- Unguarded entry points.
importcontains Loro’s panics and poisons the document (doc.rs:277). Three other entry points pass bytes from other replicas to Loro without that guard:VersionVector::decode(doc.rs:75),export_updates(:263) andcheckout(:312). - No preset for another member’s snapshot. The import presets cover an update (no snapshots) and a snapshot this device wrote itself (values not inspected) (import.rs:38, :47). None fits a snapshot made by another member, which the threat model’s flow F8 imports.
- Fail-open counting. The check for deeply nested values passes every kind of operation it does not list (import.rs:221), and an inconsistent span counts as zero operations (:172).
- Peer IDs are not tied to the sender.
importdoes not know who sent a blob, and nothing ties a change’s Loro peer identifier to the authenticated MLS sender. A collaborator who may edit could therefore write changes under another member’s peer identifier. Loro identifies changes by peer and counter, so a forged change that takes another member’s next counter could make replicas discard that member’s real change as a duplicate: a silent, permanent divergence. This is inferred from Loro’s design; it was not reproduced in this review.
Why it matters.
- ADR-0008 decision 4.
- AGENTS.md §6: every network message is hostile.
- The threat model’s T7 and F6.10 take the author of a change to be the sender whose MLS signature it carries.
Known: partly. The CORE-004 report §8.3 covers pending changes and the cost of overlapping marks.
Recommended fix.
S, in CORE-101:
- one guard for every entry point that takes bytes from others;
- an import preset for others’ snapshots, with value inspection;
- an exhaustive, fail-closed value check with a test for each kind;
- refusing inconsistent spans.
A decision, in an ADR-0016 or ADR-0008 amendment or in SRV-104’s sync protocol: bind each device’s peer identifier to its identity, and refuse imported changes whose peers are not the sender’s. Snapshots from editors remain an accepted residual risk.
M3. bayan-xml exceeds its documented memory bound on two crafted inputs
What we found. Two inputs keep far more memory than the input itself:
- The XML declaration. The reader collects every field of the declaration before checking it (source.rs:144), empty names included. Each 4 bytes of input keep about 40 bytes.
- Runs of text. A run of text keeps an entry for every piece (reader.rs:941), and
<is a new piece every 4 bytes.
Reproduced in this review with 32 MiB parts, against 0.1 MB for plain text:
- a long declaration added 328 MB, 10 times the input, before it was refused;
- a run of
<added 205 MB, 6.4 times the input.
At the 256 MiB part limit that is about 2.6 GiB and 1.6 GiB. The crate documents 768 MiB as its worst case (lib.rs:74). bayan-opc’s 100:1 limit on the compression ratio lets such a part travel in a .docx of a few megabytes, and the web engine has at most 4 GB of memory (ADR-0014 decision 8).
Why it matters. ADR-0006 decision 5, and the crate’s own documented bound, which the stays_linear_* tests do not check because they measure time only.
Known: no.
Recommended fix (S, bayan-core).
- Refuse a fourth declaration field as soon as it is read.
- Decode runs of references as they are read.
- Add regression tests that measure peak memory in a child process.
M4. Two interim parsers outlived their replacements
What we found.
- bayan-opc’s own XML parser. bayan-opc keeps its own XML parser for its metadata parts, 1,281 lines. Its documentation says that once
bayan-xmlexists, “this module is deleted” (xml.rs:3). bayan-xml was merged on 2026-10-17, while bayan-opc’s crate documentation still says it “is still to be built” (lib.rs:71). - bayan-lab’s scanner. The Fidelity Lab’s scanner has its own ZIP reader, CRC-32, package reader and XML reader, about 2,000 lines in
lab/bayan-lab/src/scan/. They are called “minimal stand-ins, … marked for replacement” and have no fuzz targets (lab/README.md:43). The lab’s S3 listing is parsed with that XML reader too. - What comes next. LAB-101 will scan thousands of documents found on the open web (private-corpus-options.md:28). The lab’s reader, not the engine’s, then decides which documents enter the corpus.
Why it matters. AGENTS.md §6 and ADR-0025 decision 4 ask every parser to be fuzzed. ADR-0002 decision 6’s rule against writing the same logic twice applies within a repository just as much. LAB-001’s merge recorded the move as a follow-up, and no work package owns it.
Known: yes, in code comments and the lab’s README.
Recommended fix (M, bayan-core, before LAB-101 starts collecting).
- Move bayan-opc’s metadata parsing onto bayan-xml, keeping its node budget, and delete
xml.rs. - Move the lab’s scanner onto bayan-opc and bayan-xml, keeping
tests/lab/hostile_packages.rsas its acceptance tests.
M5. The outside-world rule is enforced only by review, and the engine already reads the environment
What we found.
- One environment read today. The engine starts its thread without a stack size (thread.rs:166). Rust’s standard library then takes the size from the
RUST_MIN_STACKenvironment variable. The depth at which a deeply nested input overflows the stack therefore depends on the environment; CORE-004 measured its stack-overflow finding at the 2 MiB default. - What the lints cover.
clippy.tomlforbids only floating-point methods and hash collections (clippy.toml:9, :81). Nothing rejects clocks, environment variables, files, sockets or processes in the engine’s crates, and nothing checks what the engine’s WebAssembly module imports from its host. - Platform calls arriving through dependencies:
- cfb brings
web-time, a clock, on wasm32; - Loro’s randomness and clock are an accepted exception;
- OpenMLS reads the clock and the platform’s randomness (threat model §13.3 item 2).
- cfb brings
- The planned check comes late. ADR-0012’s amendment asks for a check of the imports only when
bayan-crdtis first linked.
Why it matters. ADR-0012 decision 5. ADR-0005 decision 5: no clock, locale, environment or thread scheduling in output. plan/03 §12: the engine “reads no environment variables”.
Known: the import check is planned for later; the rest is not recorded.
Recommended fix (S, bayan-core).
- Give the engine’s thread an explicit stack size.
- Make
cargo xtask wasm-packagefail on any module import that is not on a reviewed list; today that list holds only wasm-bindgen’s glue. - Add a gate check that rejects
std::time,std::env,std::fs,std::netandstd::processin every crate except the binding crates and the command-line tool.
M6. Neither shell recovers from an engine panic as the protocol says
What we found.
- bayan-desktop ends the session for good after an unrecoverable
engine.error(documentsession.cpp:147). - bayan-web starts a new session at once, with no limit, and shows nothing for a panic (document-controller.ts:166, :254).
- With real documents (DESK-101, WEB-101), a file that makes the engine panic when it opens would stop the desktop and send the web app into an endless, silent loop. Neither shell has a test that uses the engine’s
diag.panicswitch.
Why it matters. Spec §12: after an unrecoverable error “the shell must start a new one with hello”. ADR-0006 decision 4: the engine “offers recovery from the auto-recovery journal”.
Known: no.
Recommended fix (S, in both shells, and one sentence in spec §12).
- One policy for both shells: restart a limited number of times, tell the user, and never reopen the document that caused the panic without asking.
- An end-to-end test in each shell with
test.allow_panicanddiag.panic.
M7. The desktop’s tests and sanitizers miss the real engine boundary
What we found.
- The stand-in engine is out of date. The default build links a stand-in engine whose C header still says “PROVISIONAL … until that SDK exists” (bayan_ffi.h:4). It lacks
BAYAN_STATUS_WRONG_THREAD, and it renders tiles on the caller’s thread, where the real engine renders them on its own thread in order with messages. - The sanitizers never meet the real engine. The AddressSanitizer and UndefinedBehaviorSanitizer job builds only with that stand-in (CMakePresets.json:11, ci.yml:112). The riskiest code, where the Rust engine thread calls into C++, never runs under them.
- Hand-made messages disagree with the schema. The tests build engine messages by hand, and some disagree with the generated schema:
engine.erroris built with its fields nested undererror(tst_documentsession.cpp:318), which the real engine never sends. The SDK’s JSON Schema is shipped but never read (src/engine/README.md:13).
Why it matters. AGENTS.md §4. ADR-0012 decision 1: one schema, generated from the engine. plan/07 counts sanitizers among its dynamic analyses.
Known: partly. The README calls the stand-in a stand-in.
Recommended fix (M, bayan-desktop).
- Build the core SDK in the sanitizer job, and run the engine and app tests under the sanitizers against it.
- Replace the stand-in’s header with the SDK’s, and keep the stand-in only for its error-path switches.
- Take incoming test messages from the real engine’s output, or check them against the schema.
M8. Two lockfiles that CI compiles are outside the automatic 24-hour check
What we found.
- bayan-core’s fuzz workspace.
check-exact-pinsandcheck-lockfile-agecannot read its lockfile, so its age is checked by hand (fuzz/README.md:33)./fuzz/, which also holds the NCSA license exception, is not inCODEOWNERS. - bayan-server’s MLS spike. Its lockfile is likewise checked by hand; its AGENTS.md names the one-line fix, passing
--all-featurestocargo metadata. - Code from these packages runs. In both cases the packages’ build scripts run in CI and in agent sessions.
Why it matters. ADR-0017 rule 4 asks for the age rule to be “enforced by tooling wherever possible”, and threat T16 covers malicious dependencies.
Known: yes, in both repositories’ documentation.
Recommended fix (S, xtask/src/supply_chain/ in both repositories together).
- Pass
--all-featurestocargo metadata. - Let the check accept path dependencies from the fuzz workspace into the main workspace’s crates.
- Add
/fuzz/to bayan-core’sCODEOWNERS.
M9. No automated accessibility checks, and the manual validation passes are still open
What we found.
- No automated checks. ADR-0014 decision 3 names axe rules for automated accessibility checks, and ADR-0020 decision 6 says that “automated accessibility checks run on every pull request”. bayan-web has no rule-based checks such as axe, and bayan-desktop’s tests cover its accessibility bridge but check no accessibility rules for its interface.
- The validation gates are open. ADR-0013’s and ADR-0014’s gates still need NVDA, VoiceOver, real input methods on Windows and macOS and in Firefox and Safari, and frame times on real Macs; the owner deferred them on 2026-10-14. DESK-103 and WEB-102 will build on accessibility designs that no screen reader besides Orca has read.
- The language gap. The accessibility tree’s missing language and direction is part of H5.
Known: yes; the ADRs record the open passes.
Recommended fix.
- S, bayan-web: axe rules in the Playwright tests. That is a new development dependency, so its pull request justifies it under ADR-0017.
- Owner: run the scripted manual passes before DESK-103 and WEB-102 are refined to Ready.
M10. PostgreSQL: the plan lets account data arrive before TLS, and the TLS defaults are not fixed
What we found.
- Unencrypted today, as documented. The server talks to PostgreSQL without encryption today, by design and documented (bayan-db-postgres lib.rs:5), because TLS needs the C code that ADR-0028 accepted later.
- The order of the work packages. SRV-101 (accounts, sessions and devices) and SRV-102 (which adds TLS to PostgreSQL) both depend only on SRV-001 and SRV-003, so account data can reach PostgreSQL before TLS does.
- Weak modes are accepted. The URL check accepts weak
sslmodevalues such asprefer(tested in lib.rs:315). Once TLS exists,preferfalls back to plain text without verifying anything.
Why it matters. ADR-0028 decision 3: certificates are always verified, sslmode=verify-full.
Known: the plain-text gap, yes; the order and the defaults, no.
Recommended fix (S).
- At the Phase-0 gate: SRV-101 follows SRV-102’s TLS part, or that part becomes a small first work package. It would unban
aws-lc-sys, add a CA bundle to the image and its smoke test, and add thewebpki-rootslicense exception. - In SRV-102: use
verify-fullunless the URL names a loopback host, and refuse weakersslmodevalues, with tests.
M11. The server’s two migration copies are kept in step by a comment
What we found. Every schema change is written twice, once for SQLite and once for PostgreSQL. The only safeguard is a “Keep in step” comment at the top of each migration (SQLite, PostgreSQL). The CI check of the query metadata compares each crate with its own migrations only. SRV-102 is about to add the whole metadata schema.
Recommended fix (S, bayan-server). Add a test that compares the two backends’ migrations (versions and descriptions) and their migrated schemas (tables, columns and indexes).
M12. Nothing records that Loro’s crash findings reached its maintainers
What we found.
- The report. CORE-004 found crafted updates that make Loro panic or abort. Its report called reporting them privately “urgent”, because the committed fuzzer regenerates them within minutes and one reproduction is a committed test fixture.
- The status. The owner sends these reports (ADR-0008, decision 4; bayan-core issue 14). Nothing in this repository records that they were sent, and the report still marks its fuzzing criterion “Partly met”. This review could not read issue 14.
Recommended fix (S, owner). Confirm in issue 14 when the reports went out. A small docs change then records it next to ADR-0008’s conditions.
Low
L1. Loro is imported outside bayan-crdt, in tests
bayan-model’s tests use Loro directly through a development dependency (Cargo.toml:16, foreign.rs:7), to play a replica that stores values the adapter refuses. ADR-0008 decision 1 says that no crate but bayan-crdt imports Loro directly. Fix (S): give bayan-crdt a test helper that writes such values, or record the test-only exception in ADR-0008; then add a gate check that only bayan-crdt names Loro.
L2. The shared supply-chain checker is GPL in one repository and AGPL in the other
xtask/src/supply_chain/ is byte-identical in bayan-core and bayan-server and must be changed in both together, but it inherits each repository’s default license: GPL-3.0-or-later with the App Store Permission in one, AGPL-3.0-or-later in the other. ADR-0003’s amendment of 2026-10-06 made the shared .github/ tooling MIT-0 for exactly this reason: a fix contributed to one copy could not be copied into the other without its contributor’s permission. Fix (S, owner, because licensing is the owner’s decision): extend the MIT-0 area to xtask/src/supply_chain/** in both REUSE.toml files while the owner is still the only copyright holder.
L3. Workflow and setup hygiene
- Many jobs ask for
contents: readwithout the comment that says why, which the workflow conventions require. Examples: every job of bayan-desktop’sci.yml, theverifyjob of bayan-web’sci.yml, the jobs of bayan-server’sci.yml, and bayan-core’sverify.ymlandartifacts.yml. - bayan-web’s
ci.ymlcancels runs in progress for every event (ci.yml:21) and, alone among the workflows that do so, also runs on a schedule, so its nightly run and a push tomaincan cancel each other. bayan-desktop’sci.yml, this repository’sverify.ymland every repository’sreuse.ymlalso cancel for every event, so two quick pushes tomaincancel each other’s runs there. The other workflows cancel only runs of pull requests, or none. - bayan-server’s
scripts/dev-setup.shaccepts a tool already onPATHthat prints the pinned version, without checking its hash. bayan-desktop’s accepts any Qt installation whose version matches. - bayan-desktop’s
CODEOWNERSmarks only.github/andsrc/engine/as security-relevant, and leaves out its Qt installer, which parses downloaded archives, its engine build script anddeps/.
Fix (S): with the next change to each repository’s CI.
L4. Latent weaknesses in the shells
None of these can be exploited today; each is cheap to fix before Phase 1 shows real documents.
- bayan-desktop shows engine strings in QML labels with automatic rich-text detection (Main.qml:38). Document strings such as missing font names would let a document render markup there, and an
<img>in it could fetch a remote image (threat T18). SettextFormat: Text.PlainTexton every label that shows engine data. - bayan-web’s accessibility mirror and input bridges lack
translate="no", so a browser’s page translation could send document text to a translation service. The bridges already switch off spell checking for the same reason. - bayan-web checks only the envelope of what the worker sends. Its protocol version constant is a plain number rather than the generated type, so a protocol change would not fail type checking. The mirror’s recursion has no depth limit, and rebuilding an ancestor of the focused element would move it and lose focus.
- Tile-grid arithmetic exists in both shells and already differs in its rounding tolerance:
1e-9in tiles.ts and1e-6in tilegrid.cpp. Fix the rule in spec §7 with test vectors that both shells run.
L5. Server hardening before the Phase-1 endpoints
Fix (S each, in SRV-101 to SRV-103).
- One request-body limit applies to every route, up to 1 GiB (http/mod.rs:57). Endpoints will need their own limits, with a small default.
- Each connection gets its own task with no cap on how many (http/serve.rs:58).
- No panic hook is installed, so a panic’s message goes to standard error as plain text, outside the structured log.
- A mistyped
BAYAN_WEB_DIRis accepted without a warning.
L6. Engine boundary details
Fix (S): CORE-125 and the spec pull request of H5.
- Non-UTF-8 bytes slip through. A message that is not valid UTF-8 is accepted when the bad bytes sit in a field the engine does not know (envelope.rs:136); spec §12 says such a message is
invalid_message. - Replays cannot tell builds apart. Every build reports engine version
0.0.0(engine.rs:35), so a replay cannot tell builds apart. And because the whole recording is parsed before the versions are compared (recording.rs:339), a recording whose session state changed shape fails asinvalid_recordingrather thanrecording_mismatch. The two shells already pin different commits. - Part of the TypeScript is written by hand. The envelope, configuration and worker control types are a hand-written string in the generator (typescript.rs:276), and the configuration is not in the JSON Schema.
- Content can leak through debugging output. Message types that carry content, such as typed text, derive
Debug(protocol.rs:637). A recording’sinitial_stateserializes the whole session, so new session state reaches recordings without review (threat T36). - Protocol geometry does not use
Blu. It uses its own integer rectangle rather than bayan-units’ type. - The worker host has three weak points. It restarts after a panic through an experimental flag of wasm-bindgen (artifacts.rs:380). Its origin check compares serialized origins, which match for two opaque origins (bayan-worker.js:74). Its
restart_failedpath has no test. - The spec lags the code. It lists neither the
args.limitvaluesblob_size,blobs,tile_size,documents,text_length,composition_lengthandrecording, nor the viewport coordinate limit of ±10⁹.
L7. Parser details
Fix (S each).
- The ZIP end-record check is narrower than its documentation. The ZIP reader considers only end-of-directory records whose comment ends exactly at the end of the input (zip/read.rs:426). Other tools take the last record they find, so a crafted archive can look different to them, although the module promises that every tool sees the same. Refuse any end-record signature after the chosen record.
- Class IDs come back in the wrong byte order.
CompoundEntry::class_iddocuments the 16 bytes stored in the file, but returns them in RFC 4122 byte order, which cfb converts to (compound_file.rs:74). - No lints guard against panics on hostile input. Freedom from panics rests on convention: the workspace lints warn on
unwrap()but not on indexing or slicing, which can panic too. Deny those lints in the parser crates before Phase 1 adds more parsers. - One dependency’s
unsafecode is accepted without a record. memchr, under quick-xml, runs SIMDunsafecode on every byte of every part. Record it as accepted, as uuid’s was, or write down the rule for third-partyunsafecode on parsing paths. - Geometry arithmetic is unchecked. bayan-units’ geometry types add and subtract without checks, as they document, and release builds wrap silently. Add checked variants before CORE-113 builds layout on values from documents.
L8. Leftovers of finished spikes
- The Automerge comparison spike is still built. It is still a workspace member and keeps 14 packages in the build and in every audit. One of them is the pre-release
hexane 1.0.0-alpha.5(Cargo.lock:843). Drop it from the workspace and keep its report and results. - The CRDT spike’s fuzzer has a stale comment. It still says libFuzzer’s license awaits approval.
L9. Qt’s archive hashes come from Qt’s server at install time
bayan-desktop’s Qt installer checks each archive against the SHA-256 that download.qt.io publishes at install time (install-qt.py:341), the trust model that ADR-0017’s amendment accepted. The later workflow conventions ask for hashes “written in the repository”, so a changed archive on Qt’s server would pass without any difference showing in Git. Fix (S, monthly dependency session): record the archive hashes in deps/qt.json and require both to match.
L10. Nothing checks that the shared CI files stay identical
The shared workflows, their pinned tools and the supply-chain checker are byte-identical across the five repositories today, but only review keeps them so. Fix (S): a scheduled job in this repository that compares the shared files of the five public repositories and fails on a difference, or a step in the monthly dependency session.
L11. Documentation drift
Fix (S): one docs pull request, plus small changes in the code repositories.
- Threat model. T1 and T3 say “in place for bayan-opc; planned for the rest”, but bayan-xml’s limits are in place too. Their fuzzing mitigation does not run in CI (H2).
- ADR-0005. It says that an
i64of layout units covers “about five million kilometres” (ADR-0005); the true range is about 128 million kilometres. This needs an erratum. - Work package briefs. The briefs of 18 work packages that are Done still show “Ready” or “In review” in their own status row. plan/06 keeps status in the index, so either drop the row from the template or update it with the status pull request.
- Claims about the “Source code” link. bayan-web’s and bayan-server’s AGENTS.md say to keep a “Source code” link working that does not exist yet; WEB-103 and SRV-105 add it.
- The server’s README promises the web app in the image (H4).
- bayan-desktop’s leftovers. Its stand-in header and
.clang-tidystill describe the time before CORE-007. - bayan-opc’s documentation says bayan-xml “is still to be built” (M4).
- bayan-xml’s limits. Its documentation says bayan-opc’s 512 MiB part limit “comes first in practice”, but bayan-xml’s own 256 MiB limit is the one that binds. CORE-102 should compare a part’s declared size with it before decompressing.
- The fuzz README’s first line (H2).
What is in good shape
-
Layering. bayan-units, bayan-opc and bayan-xml depend only on third-party crates. bayan-model depends on bayan-crdt, in the same layer. bayan-engine depends only on serde, serde_json and schemars, and the binding crates only on the engine. No product crate depends on a spike or on the Fidelity Lab, and the lab is not in any shipped artifact.
-
Memory safety.
#![forbid(unsafe_code)]holds in every crate except bayan-ffi and bayan-wasm. In bayan-ffi everyunsafeblock is marked with#[expect(unsafe_code, reason = …)], and each has a// SAFETY:comment that holds. Every C entry point catches panics, and the panic hook keeps engine panics silent. -
Determinism. Output code has no hash maps and no platform floating-point math; both bans are lints at the
forbidlevel, proven by a lint canary. bayan-units’ functions are tested natively and in WebAssembly against independent references. Loro’s maps and mark sets leave the adapter sorted, its undo clock is neutralized, and the host’s peer identifier replaces Loro’s random one. -
Engine protocol v0. The engine implements its spec closely:
- the handshake;
- envelopes that refuse duplicate keys;
- every limit of §13, on outgoing messages too;
- error arguments without content;
- recordings that the engine never writes to disk, replayed with the replaying engine’s own configuration.
-
Parsers. The ZIP reader:
- checks entry counts before allocating;
- refuses path traversal, names that differ only by case or encoding, overlapping entries, encrypted entries and links;
- limits the compression ratio.
Both XML parsers refuse document type declarations and every entity but the five predefined ones, and check depth, attributes and names before doing the work. No parser error carries document text.
-
Shells. Neither shell hit-tests, moves a caret or selection, or measures text. Qt never decodes document data. The web app:
- builds no HTML from strings;
- has exactly one Trusted Types policy, created in one module;
- sends a strict CSP and enforces Subresource Integrity with a build that fails closed;
- checks the engine’s files before starting the worker (with the gap of H5);
- compiles against the generated protocol types.
Neither shell logs content.
-
Server. The server is content-blind:
- no route reads a request body, and peer addresses are discarded;
- request IDs reveal nothing, and logging is content-free and tested at the most verbose level;
- configuration refuses unknown keys, takes secrets from files and redacts them;
- the header-read timeout and graceful shutdown are tested;
- the container is built from scratch, runs as non-root on a read-only root, and allows only listed files;
- queries are static and compile-checked, and SQLite is the only native code.
-
Supply chain and CI. Apart from the gaps listed in L3, all 38 workflows in the five repositories follow the workflow conventions:
- minimal permissions;
- no persisted credentials;
- GitHub’s own actions pinned to full SHAs;
- timeouts and concurrency groups;
- no
pull_request_target; - no event data in scripts;
merge_groupon required workflows.
The shared files are byte-identical everywhere. Exact pins, committed lockfiles, the age checks, cargo-deny, pnpm’s audit and the lockfile-integrity check are in place. Every committed test document and font has a recorded license.
-
Fidelity Lab. The lab calls
curlwithout a shell, allows only HTTPS (plain HTTP only to the local machine, for tests), never relaxes certificate checks, follows no redirects, and passes credentials on standard input. Its content-addressed store accepts only canonical keys. Private documents record no paths and stay out of statistics.
Recommended next steps
For the orchestrator, in the order of plan/12 §1: follow-ups that fix security, correctness or determinism problems come before the Ready work packages.
- Owner, now. H3’s private report to cfb’s maintainer first, then H1 (audit and apply the settings; update the checklist and issue #15), M12 (confirm Loro’s reports), L2 (license of the shared checker) and the manual passes of M9.
- Small follow-ups, one repository per session.
- bayan-core: H3, M3, M5 and
merge_groupforartifacts.yml. - bayan-server: H4’s header alignment and M11.
- bayan-core and bayan-server together: M8.
- bayan-desktop: M6 and H5’s interim key change.
- bayan-web: M6 and L4’s
translate="no". - docs: H1’s required checks and L11.
- bayan-core: H3, M3, M5 and
- Work packages (M). H2 (fuzzing in CI and the missing targets), M4 (the interim parsers, before LAB-101), M7 (the desktop’s sanitizers against the real engine) and M9’s axe rules.
- Phase-0 gate (planning session).
- H5: the protocol pull request and CORE-125’s scope.
- H6: the ADR-0006 amendment on budgets and cancellation.
- H7: the
bayan-mlswork package and the decision on Git dependencies. - M1: CORE-101’s obligations and owners for isolation, restart and the import check.
- M2: binding peer identifiers to devices.
- M10: the order of SRV-101 and SRV-102 and the TLS defaults.
- The second half of H4: one source for the web app’s headers, and the deployments that exist.
- Low items. Batch them into the next change that touches each repository.
Limits of this review
- Reading only. The review was read-only. It built and ran only a small measurement program outside the repositories, for H3 and M3. It ran no fuzzer and repeated no manual test.
- What could not be read. From this session, the code repositories’ rulesets and issues could not be read. H1’s statement about their rulesets and M12’s status are therefore unconfirmed.
- What was read lightly. The spikes were read for what carries over into production code. The gate machinery of
xtaskwas read only where it bears on a finding. - The findings stand at the reviewed commits. Fixes made after them are not reflected.