diff --git a/docs/CONTRACTS.md b/docs/CONTRACTS.md index 5a62716e5..1f5dba229 100644 --- a/docs/CONTRACTS.md +++ b/docs/CONTRACTS.md @@ -87,6 +87,8 @@ Inkspan collaboration is **provider-neutral**. A host may supply Yjs-compatible Collaboration document content and awareness state are untrusted tenant data. Awareness presence is not authorization. Provider outage is a host-owned degraded mode; Inkspan must not synthesize remote durability or identity from local Yjs state. +Host awareness capability reads and listener operations are failure-contained: incompatible access reports a stable Inkspan error, collaborator counting falls back to zero, and teardown continues across failing removals without exposing host-thrown values. This containment does not make Inkspan the provider lifecycle owner. + No secret is required by the framework-independent collaboration contract itself. Provider credentials remain host-owned and must not be embedded in editor configuration or document content. ## Naruon modular composition contract diff --git a/docs/TRD.md b/docs/TRD.md index 80cec07ac..efd2b138f 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -68,6 +68,8 @@ Inkspan may bind to host-supplied Yjs-compatible document/awareness surfaces. In Provider updates and awareness state are untrusted tenant data, not authorization evidence. Provider outage/degraded mode is resolved by host policy; Inkspan must not invent durable collaboration success. +Inkspan validates the host awareness capability before use and contains host-thrown capability, collaborator-count, listener-registration, listener-removal, and teardown failures behind stable editor-owned outcomes. Cleanup still attempts every registered listener removal; these guards do not transfer provider lifecycle or transport authority to Inkspan. + ## Deterministic conversion and Office renderer Deterministic conversion is an authority boundary separate from model-assisted authoring. Model output may become a proposed document change, but deterministic editor/conversion validation decides whether the resulting content is structurally acceptable. diff --git a/src/collaboration/awareness.ts b/src/collaboration/awareness.ts index 0444244d4..6525b580f 100644 --- a/src/collaboration/awareness.ts +++ b/src/collaboration/awareness.ts @@ -19,11 +19,45 @@ export interface ScopedCollaborationProvider extends CollaborationProviderLike { dispose(): void; } +interface ScopedListenerWrapper { + callback: (...args: unknown[]) => void; + deactivate(): void; +} + const CURSOR_COLOR_PATTERN = /^#[0-9a-fA-F]{6}$/; const NUMERIC_IDENTIFIER_PATTERN = /^\d+$/; const FALLBACK_CURSOR_COLOR = '#475569'; const MAX_CURSOR_LABEL_LENGTH = 80; +/** Read and validate the host-owned awareness capability without leaking failures. */ +function readCompatibleCollaborationAwareness( + provider: CollaborationProviderLike, +): CollaborationAwareness { + try { + const awareness = provider.awareness as + | Partial + | undefined; + if ( + awareness !== undefined && + typeof awareness.clientID === 'number' && + awareness.states instanceof Map && + typeof awareness.getLocalState === 'function' && + typeof awareness.getStates === 'function' && + typeof awareness.setLocalStateField === 'function' && + typeof awareness.on === 'function' && + typeof awareness.off === 'function' + ) { + return awareness as CollaborationAwareness; + } + } catch { + // Normalize host capability access failures at the public Inkspan boundary. + } + + throw new Error( + 'collaboration provider must expose a compatible Yjs awareness instance', + ); +} + /** Validate and serialize the only public fields permitted in awareness. */ export function serializeCollaborationUser( user: CollaborationUser, @@ -60,23 +94,7 @@ export function assertCollaborationConfiguration( } if (!provider) return; - const awareness = provider.awareness as - | Partial - | undefined; - if ( - !awareness || - typeof awareness.clientID !== 'number' || - !(awareness.states instanceof Map) || - typeof awareness.getLocalState !== 'function' || - typeof awareness.getStates !== 'function' || - typeof awareness.setLocalStateField !== 'function' || - typeof awareness.on !== 'function' || - typeof awareness.off !== 'function' - ) { - throw new Error( - 'collaboration provider must expose a compatible Yjs awareness instance', - ); - } + readCompatibleCollaborationAwareness(provider); } /** @@ -86,10 +104,10 @@ export function assertCollaborationConfiguration( export function createScopedCollaborationProvider( provider: CollaborationProviderLike, ): ScopedCollaborationProvider { - const source = provider.awareness; + const source = readCompatibleCollaborationAwareness(provider); const listenerWrappers: Record< CollaborationAwarenessEvent, - Map<(...args: unknown[]) => void, (...args: unknown[]) => void> + Map<(...args: unknown[]) => void, ScopedListenerWrapper> > = { change: new Map(), update: new Map(), @@ -108,14 +126,38 @@ export function createScopedCollaborationProvider( source.setLocalStateField(field, value), on: (event, listener) => { if (listenerWrappers[event].has(listener)) return; - const wrapper = (...args: unknown[]) => listener(...args); - listenerWrappers[event].set(listener, wrapper); - source.on(event, wrapper); + let active = true; + const wrapper = (...args: unknown[]) => { + if (active) listener(...args); + }; + const deactivate = () => { + active = false; + }; + try { + source.on(event, wrapper); + } catch { + deactivate(); + try { + source.off(event, wrapper); + } catch { + // The host may reject rollback too; preserve the stable public error. + } + throw new Error('collaboration awareness listener registration failed'); + } + listenerWrappers[event].set(listener, { + callback: wrapper, + deactivate, + }); }, off: (event, listener) => { const wrapper = listenerWrappers[event].get(listener); if (!wrapper) return; - source.off(event, wrapper); + wrapper.deactivate(); + try { + source.off(event, wrapper.callback); + } catch { + throw new Error('collaboration awareness listener removal failed'); + } listenerWrappers[event].delete(listener); }, }; @@ -128,7 +170,13 @@ export function createScopedCollaborationProvider( disposed = true; for (const event of ['change', 'update'] as const) { for (const wrapper of listenerWrappers[event].values()) { - source.off(event, wrapper); + wrapper.deactivate(); + try { + source.off(event, wrapper.callback); + } catch { + // Host-owned listener teardown must not abort remaining cleanup or + // leak a private provider failure through React effect disposal. + } } listenerWrappers[event].clear(); } @@ -136,25 +184,30 @@ export function createScopedCollaborationProvider( }; } -/** Count remote awareness clients carrying a valid public user identifier. */ +/** Count remote awareness clients without leaking host awareness failures. */ export function countRemoteCollaborators( awareness: CollaborationAwareness | undefined, ): number { if (!awareness) return 0; - let count = 0; - for (const [clientId, state] of awareness.getStates()) { - if (clientId === awareness.clientID) continue; - const user = state.user; - if ( - typeof user === 'object' && - user !== null && - typeof (user as Record).id === 'string' && - (user as Record).id !== '' - ) { - count += 1; + try { + const localClientId = awareness.clientID; + let count = 0; + for (const [clientId, state] of awareness.getStates()) { + if (clientId === localClientId) continue; + const user = state.user; + if ( + typeof user === 'object' && + user !== null && + typeof (user as Record).id === 'string' && + (user as Record).id !== '' + ) { + count += 1; + } } + return count; + } catch { + return 0; } - return count; } /** Convert a host connection state into concise status-region text. */ diff --git a/src/collaboration/awarenessCountFailureContainment.test.ts b/src/collaboration/awarenessCountFailureContainment.test.ts new file mode 100644 index 000000000..b42272b16 --- /dev/null +++ b/src/collaboration/awarenessCountFailureContainment.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from 'vitest'; +import { countRemoteCollaborators } from './awareness.js'; +import type { CollaborationAwareness } from './types.js'; + +function baseAwareness(): CollaborationAwareness { + const states = new Map>(); + return { + clientID: 11, + states, + getLocalState: () => null, + getStates: () => states, + setLocalStateField: () => undefined, + on: () => undefined, + off: () => undefined, + }; +} + +describe('remote collaborator count failure containment', () => { + it('fails closed without leaking getStates failures', () => { + const privateFailure = { secret: 'provider-get-states-private' }; + const awareness = { + ...baseAwareness(), + getStates: () => { + throw privateFailure; + }, + }; + + let observed: unknown; + let count: number | undefined; + try { + count = countRemoteCollaborators(awareness); + } catch (error) { + observed = error; + } + + expect(observed).toBeUndefined(); + expect(count).toBe(0); + }); + + it('fails closed without leaking clientID access failures', () => { + const privateFailure = { secret: 'provider-client-id-private' }; + const states = new Map>([ + [12, { user: { id: 'remote-one' } }], + ]); + const awareness = { + ...baseAwareness(), + states, + getStates: () => states, + } as CollaborationAwareness; + Object.defineProperty(awareness, 'clientID', { + enumerable: true, + get() { + throw privateFailure; + }, + }); + + let observed: unknown; + let count: number | undefined; + try { + count = countRemoteCollaborators(awareness); + } catch (error) { + observed = error; + } + + expect(observed).toBeUndefined(); + expect(count).toBe(0); + }); +}); diff --git a/src/collaboration/awarenessDisposeFailureContainment.test.ts b/src/collaboration/awarenessDisposeFailureContainment.test.ts new file mode 100644 index 000000000..ac95671af --- /dev/null +++ b/src/collaboration/awarenessDisposeFailureContainment.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createScopedCollaborationProvider } from './awareness.js'; +import type { + CollaborationAwareness, + CollaborationAwarenessEvent, +} from './types.js'; + +describe('scoped collaboration provider cleanup containment', () => { + it('attempts every listener detachment without leaking host cleanup failures', () => { + const privateFailure = new Error('sensitive-provider-cleanup-internal'); + const sourceListeners: Record< + CollaborationAwarenessEvent, + Set<(...args: unknown[]) => void> + > = { + change: new Set(), + update: new Set(), + }; + let offCalls = 0; + const source: CollaborationAwareness = { + clientID: 7, + states: new Map(), + getLocalState: () => null, + getStates: () => new Map(), + setLocalStateField: () => undefined, + on: (event, listener) => sourceListeners[event].add(listener), + off: (event, listener) => { + offCalls += 1; + if (event === 'change') throw privateFailure; + sourceListeners[event].delete(listener); + }, + }; + const scoped = createScopedCollaborationProvider({ awareness: source }); + const changeListener = vi.fn(); + const updateListener = vi.fn(); + + scoped.awareness.on('change', changeListener); + scoped.awareness.on('update', updateListener); + + let observed: unknown; + try { + scoped.dispose(); + } catch (error) { + observed = error; + } + + expect(observed).toBeUndefined(); + expect(offCalls).toBe(2); + expect(sourceListeners.update.size).toBe(0); + expect(sourceListeners.change.size).toBe(1); + + for (const retainedHostListener of sourceListeners.change) { + retainedHostListener({ stale: true }); + } + expect(changeListener).not.toHaveBeenCalled(); + + scoped.dispose(); + expect(offCalls).toBe(2); + }); +}); diff --git a/src/collaboration/awarenessListenerContainment.test.ts b/src/collaboration/awarenessListenerContainment.test.ts new file mode 100644 index 000000000..ec65e9aa0 --- /dev/null +++ b/src/collaboration/awarenessListenerContainment.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createScopedCollaborationProvider } from './awareness.js'; +import type { CollaborationAwareness } from './types.js'; + +function awarenessWithListenerRegistrationFailure(): { + awareness: CollaborationAwareness; + registrationAttempts: () => number; +} { + const states = new Map>(); + let attempts = 0; + const privateFailure = new Error('private provider listener registration failure'); + const awareness: CollaborationAwareness = { + clientID: 11, + states, + getLocalState: () => null, + getStates: () => states, + setLocalStateField: () => undefined, + on: () => { + attempts += 1; + if (attempts === 1) throw privateFailure; + }, + off: () => undefined, + }; + return { awareness, registrationAttempts: () => attempts }; +} + +function awarenessWithListenerRemovalFailure(): { + awareness: CollaborationAwareness; + removalAttempts: () => number; +} { + const states = new Map>(); + let attempts = 0; + const privateFailure = new Error('private provider listener removal failure'); + const awareness: CollaborationAwareness = { + clientID: 11, + states, + getLocalState: () => null, + getStates: () => states, + setLocalStateField: () => undefined, + on: () => undefined, + off: () => { + attempts += 1; + if (attempts === 1) throw privateFailure; + }, + }; + return { awareness, removalAttempts: () => attempts }; +} + +describe('scoped collaboration provider listener containment', () => { + it('removes a listener when the host registers it before throwing', () => { + const states = new Map>(); + const listeners = new Set<(...args: unknown[]) => void>(); + const awareness: CollaborationAwareness = { + clientID: 11, + states, + getLocalState: () => null, + getStates: () => states, + setLocalStateField: () => undefined, + on: (_event, listener) => { + listeners.add(listener); + throw new Error('private post-registration failure'); + }, + off: (_event, listener) => { + listeners.delete(listener); + }, + }; + const scoped = createScopedCollaborationProvider({ awareness }); + const listener = vi.fn(); + + expect(() => scoped.awareness.on('change', listener)).toThrowError( + new Error('collaboration awareness listener registration failed'), + ); + for (const registeredListener of listeners) registeredListener(); + + expect(listener).not.toHaveBeenCalled(); + expect(listeners.size).toBe(0); + }); + + it('deactivates a leaked listener when registration rollback also fails', () => { + const states = new Map>(); + const listeners = new Set<(...args: unknown[]) => void>(); + const awareness: CollaborationAwareness = { + clientID: 11, + states, + getLocalState: () => null, + getStates: () => states, + setLocalStateField: () => undefined, + on: (_event, listener) => { + listeners.add(listener); + throw new Error('private post-registration failure'); + }, + off: () => { + throw new Error('private rollback failure'); + }, + }; + const scoped = createScopedCollaborationProvider({ awareness }); + const listener = vi.fn(); + + expect(() => scoped.awareness.on('change', listener)).toThrowError( + new Error('collaboration awareness listener registration failed'), + ); + for (const registeredListener of listeners) registeredListener(); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('redacts a rejected listener registration and permits a clean retry', () => { + const source = awarenessWithListenerRegistrationFailure(); + const scoped = createScopedCollaborationProvider({ awareness: source.awareness }); + const listener = vi.fn(); + + expect(() => scoped.awareness.on('change', listener)).toThrowError( + new Error('collaboration awareness listener registration failed'), + ); + expect(source.registrationAttempts()).toBe(1); + + expect(() => scoped.awareness.on('change', listener)).not.toThrow(); + expect(source.registrationAttempts()).toBe(2); + }); + + it('redacts a rejected listener removal and retains state for retry', () => { + const source = awarenessWithListenerRemovalFailure(); + const scoped = createScopedCollaborationProvider({ awareness: source.awareness }); + const listener = vi.fn(); + + scoped.awareness.on('change', listener); + expect(() => scoped.awareness.off('change', listener)).toThrowError( + new Error('collaboration awareness listener removal failed'), + ); + expect(source.removalAttempts()).toBe(1); + + expect(() => scoped.awareness.off('change', listener)).not.toThrow(); + expect(source.removalAttempts()).toBe(2); + }); +}); diff --git a/src/collaboration/awarenessProviderAccess.test.ts b/src/collaboration/awarenessProviderAccess.test.ts new file mode 100644 index 000000000..65eeec0ac --- /dev/null +++ b/src/collaboration/awarenessProviderAccess.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from 'vitest'; +import { + assertCollaborationConfiguration, + createScopedCollaborationProvider, +} from './awareness.js'; +import type { + CollaborationAwareness, + CollaborationProviderLike, +} from './types.js'; + +function validAwareness(): CollaborationAwareness { + const states = new Map>(); + return { + clientID: 11, + states, + getLocalState: () => null, + getStates: () => states, + setLocalStateField: () => undefined, + on: () => undefined, + off: () => undefined, + }; +} + +describe('collaboration provider capability access', () => { + it('does not leak a private provider error when awareness changes after validation', () => { + const awareness = validAwareness(); + let reads = 0; + const provider = Object.defineProperty({}, 'awareness', { + enumerable: true, + get() { + reads += 1; + if (reads === 1) return awareness; + throw new Error('sensitive-provider-internal'); + }, + }) as CollaborationProviderLike; + + expect(() => + assertCollaborationConfiguration(provider, undefined), + ).not.toThrow(); + expect(() => createScopedCollaborationProvider(provider)).toThrowError( + new Error( + 'collaboration provider must expose a compatible Yjs awareness instance', + ), + ); + }); + + it('normalizes private structural awareness access failures', () => { + const privateFailure = new Error('sensitive-awareness-internal'); + const awareness = Object.defineProperty({}, 'clientID', { + enumerable: true, + get() { + throw privateFailure; + }, + }) as CollaborationAwareness; + const provider = { awareness } as CollaborationProviderLike; + + let observed: unknown; + try { + assertCollaborationConfiguration(provider, undefined); + } catch (error) { + observed = error; + } + + expect(observed).toBeInstanceOf(Error); + expect(observed).not.toBe(privateFailure); + expect((observed as Error).message).toBe( + 'collaboration provider must expose a compatible Yjs awareness instance', + ); + }); +});