diff --git a/src/collaboration/awareness.ts b/src/collaboration/awareness.ts index df3a414a..677927f4 100644 --- a/src/collaboration/awareness.ts +++ b/src/collaboration/awareness.ts @@ -30,6 +30,7 @@ const FALLBACK_CURSOR_COLOR = '#475569'; const MAX_CURSOR_LABEL_LENGTH = 80; const MAX_PUBLIC_IDENTIFIER_LENGTH = 80; const MAX_LOCAL_FIELD_SOURCE_LENGTH = 1_024; +const MAX_REMOTE_FIELD_SOURCE_LENGTH = 1_024; type CollaborationUserField = 'userId' | 'displayName' | 'cursorColor'; /** Reject malformed or oversized local identity fields before normalization. */ @@ -85,6 +86,23 @@ function exceedsPublicIdentifierLength(value: string): boolean { return false; } +/** Read one own enumerable data field without invoking caller-defined accessors. */ +function ownEnumerableDataValue( + value: unknown, + property: string, +): unknown { + if (typeof value !== 'object' || value === null) return undefined; + try { + const descriptor = Object.getOwnPropertyDescriptor(value, property); + if (!descriptor) return undefined; + if (!descriptor.enumerable) return undefined; + if (!('value' in descriptor)) return undefined; + return descriptor.value; + } catch { + return undefined; + } +} + /** Read and validate the host-owned awareness capability without leaking failures. */ function readCompatibleCollaborationAwareness( provider: CollaborationProviderLike, @@ -248,7 +266,7 @@ export function createScopedCollaborationProvider( }; } -/** Count remote awareness clients without leaking host awareness failures. */ +/** Count valid remote collaborators without leaking host awareness failures. */ export function countRemoteCollaborators( awareness: CollaborationAwareness | undefined, ): number { @@ -258,15 +276,20 @@ export function countRemoteCollaborators( let count = 0; for (const [clientId, state] of awareness.getStates()) { if (clientId === localClientId) continue; - const user = state.user; + const user = ownEnumerableDataValue(state, 'user'); + if (typeof user !== 'object' || user === null) continue; + const id = ownEnumerableDataValue(user, 'id'); + if (typeof id !== 'string') continue; + if (id.length > MAX_REMOTE_FIELD_SOURCE_LENGTH) continue; + const normalizedId = id.trim(); if ( - typeof user === 'object' && - user !== null && - typeof (user as Record).id === 'string' && - (user as Record).id !== '' + normalizedId === '' || + NUMERIC_IDENTIFIER_PATTERN.test(normalizedId) || + exceedsPublicIdentifierLength(normalizedId) ) { - count += 1; + continue; } + count += 1; } return count; } catch { diff --git a/src/collaboration/awarenessIdentityCount.test.ts b/src/collaboration/awarenessIdentityCount.test.ts new file mode 100644 index 00000000..8ad38703 --- /dev/null +++ b/src/collaboration/awarenessIdentityCount.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, it, vi } from 'vitest'; +import { countRemoteCollaborators } from './awareness.js'; +import type { CollaborationAwareness } from './types.js'; + +function awarenessWith( + states: Map>, +): CollaborationAwareness { + return { + clientID: 11, + states, + getLocalState: () => states.get(11) ?? null, + getStates: () => states, + setLocalStateField: () => undefined, + on: () => undefined, + off: () => undefined, + }; +} + +describe('collaboration awareness identity counting', () => { + it('excludes remote identities that violate the public identifier contract', () => { + const states = new Map>([ + [11, { user: { id: 'local-editor' } }], + [12, { user: { id: ' ' } }], + [13, { user: { id: '12345' } }], + [14, { user: { id: `editor-${'a'.repeat(74)}` } }], + [15, { user: { id: 'editor-bob' } }], + [16, { user: { id: `${'a'.repeat(79)}😀` } }], + ]); + + expect(countRemoteCollaborators(awarenessWith(states))).toBe(2); + }); + + it('rejects oversized remote identifiers before normalization', () => { + const oversizedId = `editor-${'a'.repeat(1_024)}`; + const states = new Map>([ + [11, { user: { id: 'local-editor' } }], + [12, { user: { id: oversizedId } }], + [13, { user: { id: ' editor-bob ' } }], + ]); + const trimSpy = vi.spyOn(String.prototype, 'trim'); + + try { + expect(countRemoteCollaborators(awarenessWith(states))).toBe(1); + expect( + trimSpy.mock.instances.some( + (receiver) => String(receiver) === oversizedId, + ), + ).toBe(false); + } finally { + trimSpy.mockRestore(); + } + }); + + it('skips accessor-backed remote identity fields without executing caller code', () => { + let userGetterCalls = 0; + let idGetterCalls = 0; + const accessorBackedState: Record = {}; + Object.defineProperty(accessorBackedState, 'user', { + enumerable: true, + get() { + userGetterCalls += 1; + throw new Error('private remote user getter must not execute'); + }, + }); + const accessorBackedUser: Record = {}; + Object.defineProperty(accessorBackedUser, 'id', { + enumerable: true, + get() { + idGetterCalls += 1; + throw new Error('private remote id getter must not execute'); + }, + }); + const states = new Map>([ + [11, { user: { id: 'local-editor' } }], + [12, accessorBackedState], + [13, { user: accessorBackedUser }], + [14, { user: { id: 'editor-bob' } }], + ]); + + expect(countRemoteCollaborators(awarenessWith(states))).toBe(1); + expect(userGetterCalls).toBe(0); + expect(idGetterCalls).toBe(0); + }); + + it('ignores inherited and non-enumerable remote identity fields', () => { + const inheritedState = Object.create({ + user: { id: 'editor-inherited' }, + }) as Record; + const nonEnumerableState: Record = {}; + Object.defineProperty(nonEnumerableState, 'user', { + enumerable: false, + value: { id: 'editor-hidden' }, + }); + const nonEnumerableIdUser: Record = {}; + Object.defineProperty(nonEnumerableIdUser, 'id', { + enumerable: false, + value: 'editor-hidden-id', + }); + const states = new Map>([ + [11, { user: { id: 'local-editor' } }], + [12, inheritedState], + [13, nonEnumerableState], + [14, { user: { name: 'missing-id' } }], + [15, { user: nonEnumerableIdUser }], + [16, { user: { id: 'editor-bob' } }], + ]); + + expect(countRemoteCollaborators(awarenessWith(states))).toBe(1); + }); + + it('skips null and primitive remote states before descriptor reflection', () => { + const runtimeStates = new Map([ + [11, { user: { id: 'local-editor' } }], + [12, null], + [13, 'not-an-awareness-state'], + [14, { user: { id: 'editor-bob' } }], + ]); + const states = runtimeStates as unknown as Map< + number, + Record + >; + + expect(countRemoteCollaborators(awarenessWith(states))).toBe(1); + }); + + it('skips reflection-hostile remote identity shapes without leaking failures', () => { + const hostileState = new Proxy>( + {}, + { + getOwnPropertyDescriptor() { + throw new Error('private remote descriptor trap must not escape'); + }, + }, + ); + const states = new Map>([ + [11, { user: { id: 'local-editor' } }], + [12, hostileState], + [13, { user: { id: 'editor-bob' } }], + ]); + + expect(countRemoteCollaborators(awarenessWith(states))).toBe(1); + }); +});