The Yjs read path guards its way to safety and then discards it with a cast on the last step. This is the only cast cluster sitting on a genuinely untrusted boundary -- the data is written by other people's browsers.
Current state
packages/web/src/project/localCollections.ts, project/workspace-data.ts, primitives/useProject/handlers/*, and primitives/useProject/flatKeyMigration.ts are already half-defensive: instanceof Y.Map guards, asString / asNumber coercers. Then:
// localCollections.ts:129
const type = asString(checklist.get('type'), 'AMSTAR2') as ChecklistRow['type'];
// localCollections.ts:151
value: plainValue(rawValue) as AnswerRow['value'],A peer client writing type: "GARBAGE" into the doc gets it laundered into a ChecklistRow['type'] union with no validation.
packages/web/src has 1,070 as <Type> casts overall; this path is the largest single concentration and the only one where the input is not under our control.
Approach
The April audit already prescribed the shape of this fix in its "Yjs ceiling" section, and that analysis still holds: Yjs's API genuinely cannot give better runtime typing, because Y.Map values are dynamically typed at the protocol level. Do not try to fix this with better generics.
Build a typed facade over the Y.Doc that exposes parsing read methods. Consumers get typed access; the casts either live inside the facade or become parses.
The decision, and what the code already does
Parsing will reject data that already exists in production documents. The options are: drop invalid rows, fall back to a default, or quarantine and surface them.
The read path already chose the second one, silently: asString(checklist.get('type'), 'AMSTAR2'), asString(checklist.get('status'), 'pending'), asString(outcome.createdBy, 'local'). A row with a bad type is not rejected today, it is displayed as AMSTAR 2. So the real decision is narrower: keep that fallback and make it observable, or change it.
Recommendation: keep the fallback, and have the facade count and log every fallback with project id, path, and the rejected value, so bad documents surface in Loki instead of rendering as something else. Do not drop rows: a dropped row in a CRDT disappears for every collaborator, not just the client that failed the parse. Do not build quarantine: it is a product feature nobody has asked for. Confirm or overrule this before the facade is written.
Done when
- Reads go through facade methods that parse rather than assert
- Invalid-data behavior is decided, documented, and tested
- The casts live in the facade or are gone
Effort: 1-2 days after the decision.
Design note: how cf-sync-engine avoids this problem
cf-sync-engine's @cf-sync/yjs package does not solve this and is not a drop-in, but its architecture is worth considering before building the facade.
It keeps Y.Doc contents opaque. Yjs there is field-level rich text: the engine syncs byte-level update frames (field-frames.ts, MAX_FIELD_UPDATE_BYTES, reject reasons) and never reads structure out of a document. There is no "parse a Y.Map tree into typed rows" step, so there is nothing to cast.
CoRATES is structurally different -- the project doc is a tree of Y.Maps (studies -> checklists -> answers) that gets read into StudyRow / ChecklistRow / AnswerRow -- so the reading is unavoidable here. But it raises a question worth answering explicitly in the design: how much of that tree genuinely needs to be structured CRDT data that both peers and our code interpret, versus opaque data with structure held elsewhere. A smaller structured surface is a smaller parse surface.
Part of #778 (TypeScript correctness target state and tracker).
The Yjs read path guards its way to safety and then discards it with a cast on the last step. This is the only cast cluster sitting on a genuinely untrusted boundary -- the data is written by other people's browsers.
Current state
packages/web/src/project/localCollections.ts,project/workspace-data.ts,primitives/useProject/handlers/*, andprimitives/useProject/flatKeyMigration.tsare already half-defensive:instanceof Y.Mapguards,asString/asNumbercoercers. Then:A peer client writing
type: "GARBAGE"into the doc gets it laundered into aChecklistRow['type']union with no validation.packages/web/srchas 1,070as <Type>casts overall; this path is the largest single concentration and the only one where the input is not under our control.Approach
The April audit already prescribed the shape of this fix in its "Yjs ceiling" section, and that analysis still holds: Yjs's API genuinely cannot give better runtime typing, because
Y.Mapvalues are dynamically typed at the protocol level. Do not try to fix this with better generics.Build a typed facade over the Y.Doc that exposes parsing read methods. Consumers get typed access; the casts either live inside the facade or become parses.
The decision, and what the code already does
Parsing will reject data that already exists in production documents. The options are: drop invalid rows, fall back to a default, or quarantine and surface them.
The read path already chose the second one, silently:
asString(checklist.get('type'), 'AMSTAR2'),asString(checklist.get('status'), 'pending'),asString(outcome.createdBy, 'local'). A row with a badtypeis not rejected today, it is displayed as AMSTAR 2. So the real decision is narrower: keep that fallback and make it observable, or change it.Recommendation: keep the fallback, and have the facade count and log every fallback with project id, path, and the rejected value, so bad documents surface in Loki instead of rendering as something else. Do not drop rows: a dropped row in a CRDT disappears for every collaborator, not just the client that failed the parse. Do not build quarantine: it is a product feature nobody has asked for. Confirm or overrule this before the facade is written.
Done when
Effort: 1-2 days after the decision.
Design note: how cf-sync-engine avoids this problem
cf-sync-engine's@cf-sync/yjspackage does not solve this and is not a drop-in, but its architecture is worth considering before building the facade.It keeps Y.Doc contents opaque. Yjs there is field-level rich text: the engine syncs byte-level update frames (
field-frames.ts,MAX_FIELD_UPDATE_BYTES, reject reasons) and never reads structure out of a document. There is no "parse a Y.Map tree into typed rows" step, so there is nothing to cast.CoRATES is structurally different -- the project doc is a tree of
Y.Maps (studies -> checklists -> answers) that gets read intoStudyRow/ChecklistRow/AnswerRow-- so the reading is unavoidable here. But it raises a question worth answering explicitly in the design: how much of that tree genuinely needs to be structured CRDT data that both peers and our code interpret, versus opaque data with structure held elsewhere. A smaller structured surface is a smaller parse surface.Part of #778 (TypeScript correctness target state and tracker).