From 6f88775ea24a22d15deaa76fe6516e923394e033 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Wed, 15 Jul 2026 21:52:06 +0200 Subject: [PATCH] fix(sync): authenticate LWW project-move footprint (#9053) (#9054) SuperSync E2EE (AES-256-GCM) authenticates only op.payload; envelope fields including op.entityIds travel as plaintext. The two [TASK] LWW Update meta-reducers trusted meta.entityIds (copied from that envelope) to decide which tasks to relocate, so a compromised sync server could inject victim ids and relocate arbitrary tasks. GHSA-8pxh-mgc7-gp3g. Carry the deterministic move footprint (#9001) inside the encrypted, authenticated payload as LwwUpdatePayload.projectMoveFootprint: - createLWWUpdateOp writes the footprint to both op.entityIds (the server still needs plaintext ids for its indexed conflict detection and cannot read the ciphertext) and payload.projectMoveFootprint. - convertOpToAction surfaces the authenticated copy onto meta.moveFootprint (mirrors the recreatesEntityAfterDelete pattern). - Both LWW-TASK reducers (project repair + section membership) read meta.moveFootprint via a shared parseMoveFootprint helper and no longer trust meta.entityIds. Legacy ops without the field fall back to receiving-state repair. - getTaskProjectMoveEntityIds (footprint re-derivation during conflict resolution, fed remote ops) reads the authenticated payload instead of op.entityIds, so a tampered remote envelope cannot be laundered into a freshly-authenticated merged op. Covers the disjoint-merge, local-win, and superseded-op callers via one choke point. Additive optional field: forward/backward compatible, no crypto or envelope-version change. The DELETE/archive (bulk-archive-filter) and conflict-detection (get-op-entity-ids) envelope reads are suppression- only, not relocation, and remain for the durable AAD hardening. --- packages/sync-core/src/operation.types.ts | 9 ++ .../apply/operation-converter.util.spec.ts | 35 ++++++ .../op-log/apply/operation-converter.util.ts | 18 ++- .../core/persistent-action.interface.ts | 4 + .../sync/conflict-resolution.service.spec.ts | 101 ++++++++++++++++- .../sync/conflict-resolution.service.ts | 55 +++++++--- ...erseded-operation-resolver.service.spec.ts | 38 ++++++- .../lww-update.meta-reducer.spec.ts | 103 +++++++++++++++++- .../lww-update.meta-reducer.ts | 27 ++--- .../section-shared.reducer.spec.ts | 48 +++++++- .../section-shared.reducer.ts | 15 ++- .../task-shared-helpers.ts | 24 +++- 12 files changed, 431 insertions(+), 46 deletions(-) diff --git a/packages/sync-core/src/operation.types.ts b/packages/sync-core/src/operation.types.ts index 6abd57e047..398bc7663d 100644 --- a/packages/sync-core/src/operation.types.ts +++ b/packages/sync-core/src/operation.types.ts @@ -241,6 +241,15 @@ export interface LwwUpdatePayload< * absolute and archived entities must not be resurrected by LWW updates. */ recreatesEntityAfterDelete?: true; + /** + * Authenticated move footprint: the root entity id plus the subtask ids that + * a project move relocates atomically. Carried INSIDE the (E2EE-encrypted, + * GCM-authenticated) payload so consumers do not have to trust the plaintext + * `op.entityIds` envelope, which a compromised sync server can tamper with to + * relocate arbitrary tasks (GHSA-8pxh-mgc7-gp3g). Absent on legacy ops and on + * non-project-move LWW updates; consumers fall back to receiving-state repair. + */ + projectMoveFootprint?: readonly string[]; } /** diff --git a/src/app/op-log/apply/operation-converter.util.spec.ts b/src/app/op-log/apply/operation-converter.util.spec.ts index 948b28edad..d4c799e1ca 100644 --- a/src/app/op-log/apply/operation-converter.util.spec.ts +++ b/src/app/op-log/apply/operation-converter.util.spec.ts @@ -81,6 +81,41 @@ describe('operation-converter utility', () => { expect(action.meta.opType).toBe(OpType.Create); }); + it('should surface the authenticated projectMoveFootprint onto meta.projectMoveFootprint', () => { + // The footprint rides inside the (encrypted) payload; convertOpToAction + // exposes it to reducers so they never trust the plaintext op.entityIds + // envelope. GHSA-8pxh-mgc7-gp3g. + const op = createMockOperation({ + actionType: '[TASK] LWW Update' as ActionType, + payload: { + actionPayload: { id: 'task-456', title: 'Moved' }, + entityChanges: [], + lwwUpdateMode: 'replace', + projectMoveFootprint: ['task-456', 'subtask-1'], + } as any, + }); + const action = convertOpToAction(op); + + expect((action.meta as any).projectMoveFootprint).toEqual([ + 'task-456', + 'subtask-1', + ]); + }); + + it('should not set meta.projectMoveFootprint when the LWW payload carries no footprint', () => { + const op = createMockOperation({ + actionType: '[TASK] LWW Update' as ActionType, + payload: { + actionPayload: { id: 'task-456' }, + entityChanges: [], + lwwUpdateMode: 'replace', + } as any, + }); + const action = convertOpToAction(op); + + expect((action.meta as any).projectMoveFootprint).toBeUndefined(); + }); + describe('task-time sync payload validation', () => { const createTaskTimeOp = ( payload: Record, diff --git a/src/app/op-log/apply/operation-converter.util.ts b/src/app/op-log/apply/operation-converter.util.ts index f5ce027c6b..cfc8f38b98 100644 --- a/src/app/op-log/apply/operation-converter.util.ts +++ b/src/app/op-log/apply/operation-converter.util.ts @@ -309,6 +309,7 @@ export const convertOpToAction = (op: Operation): PersistentAction => { // IMPORTANT: Spread actionPayload FIRST, then set type, to prevent entity properties // named 'type' (like SimpleCounter.type = 'ClickCounter') from overwriting the action type. + const lwwPayload = isLwwUpdatePayload(op.payload) ? op.payload : undefined; return { ...actionPayload, type: replayActionType, @@ -316,15 +317,24 @@ export const convertOpToAction = (op: Operation): PersistentAction => { isPersistent: true, entityType: op.entityType, entityId: op.entityId, + // Plaintext, UNAUTHENTICATED envelope footprint. Never trust it for an + // LWW-TASK relocation / task-set decision — use the authenticated + // meta.projectMoveFootprint instead. GHSA-8pxh-mgc7-gp3g. (Still surfaced + // here for the non-relocation consumers that legitimately read it.) entityIds: op.entityIds, opType: op.opType, isRemote: true, // Important to prevent re-logging during replay/sync - ...(isLwwUpdatePayload(op.payload) - ? { lwwUpdateMode: op.payload.lwwUpdateMode } - : {}), - ...(isLwwUpdatePayload(op.payload) && op.payload.recreatesEntityAfterDelete === true + ...(lwwPayload ? { lwwUpdateMode: lwwPayload.lwwUpdateMode } : {}), + ...(lwwPayload?.recreatesEntityAfterDelete === true ? { recreatesEntityAfterDelete: true } : {}), + // Surface the AUTHENTICATED move footprint from the encrypted payload so + // reducers can relocate task families without trusting the plaintext + // op.entityIds envelope (which a compromised server can tamper with). + // GHSA-8pxh-mgc7-gp3g. Absent on legacy/non-move ops → receiving-state repair. + ...(lwwPayload?.projectMoveFootprint !== undefined + ? { projectMoveFootprint: lwwPayload.projectMoveFootprint } + : {}), }, }; }; diff --git a/src/app/op-log/core/persistent-action.interface.ts b/src/app/op-log/core/persistent-action.interface.ts index 5126acea93..ab0fad9d4d 100644 --- a/src/app/op-log/core/persistent-action.interface.ts +++ b/src/app/op-log/core/persistent-action.interface.ts @@ -18,6 +18,10 @@ export interface PersistentActionMeta { isBulk?: boolean; // TRUE for batch operations lwwUpdateMode?: LwwUpdateMode; recreatesEntityAfterDelete?: boolean; + // Authenticated project-move footprint surfaced from the encrypted + // LwwUpdatePayload.projectMoveFootprint. Reducers relocate task families from + // THIS field, never the plaintext `entityIds` envelope. GHSA-8pxh-mgc7-gp3g. + projectMoveFootprint?: readonly string[]; } export interface PersistentAction extends Action { diff --git a/src/app/op-log/sync/conflict-resolution.service.spec.ts b/src/app/op-log/sync/conflict-resolution.service.spec.ts index 154a83e1b9..2162990abb 100644 --- a/src/app/op-log/sync/conflict-resolution.service.spec.ts +++ b/src/app/op-log/sync/conflict-resolution.service.spec.ts @@ -1,6 +1,9 @@ import { TestBed } from '@angular/core/testing'; import { SyncConflictBannerService } from './sync-conflict-banner.service'; -import { ConflictResolutionService } from './conflict-resolution.service'; +import { + ConflictResolutionService, + getLatestTaskProjectMoveEntityIds, +} from './conflict-resolution.service'; import { Store } from '@ngrx/store'; import { OperationApplierService } from '../apply/operation-applier.service'; import { HydrationStateService } from '../apply/hydration-state.service'; @@ -7623,6 +7626,102 @@ describe('ConflictResolutionService', () => { ); expect(op.entityIds).toEqual(['task-canonical', 'subtask-1']); + // The same footprint must also ride inside the authenticated payload so + // remote clients don't have to trust the plaintext envelope. + // GHSA-8pxh-mgc7-gp3g. + expect( + (op.payload as { projectMoveFootprint?: readonly string[] }).projectMoveFootprint, + ).toEqual(['task-canonical', 'subtask-1']); + }); + + it('should omit projectMoveFootprint when no footprint is supplied', () => { + const op = service.createLWWUpdateOp( + 'TASK', + 'task-canonical', + { title: 'Local winner' }, + TEST_CLIENT_ID, + { [TEST_CLIENT_ID]: 1 }, + Date.now(), + ); + + expect(op.entityIds).toBeUndefined(); + expect( + (op.payload as { projectMoveFootprint?: readonly string[] }).projectMoveFootprint, + ).toBeUndefined(); + }); + + describe('getLatestTaskProjectMoveEntityIds (authenticated footprint source)', () => { + const lwwMoveOp = (overrides: Partial): Operation => + ({ + id: 'op-x', + actionType: toLwwUpdateActionType('TASK'), + opType: OpType.Update, + entityType: 'TASK', + entityId: 'taskT', + payload: { + actionPayload: { id: 'taskT' }, + entityChanges: [], + lwwUpdateMode: 'replace', + }, + clientId: 'c', + vectorClock: { c: 1 }, + timestamp: 1, + schemaVersion: CURRENT_SCHEMA_VERSION, + ...overrides, + }) as Operation; + + it('reuses the footprint from the authenticated payload, ignoring a tampered entityIds envelope (GHSA-8pxh-mgc7-gp3g)', () => { + // A compromised server appended 'victim' to a remote LWW op's plaintext + // entityIds envelope. Re-derivation must launder nothing: the merged op's + // footprint comes from the authenticated payload.projectMoveFootprint. + const op = lwwMoveOp({ + entityIds: ['taskT', 'victim'], + payload: { + actionPayload: { id: 'taskT' }, + entityChanges: [], + lwwUpdateMode: 'replace', + projectMoveFootprint: ['taskT', 'sub1'], + } as unknown as Operation['payload'], + }); + + expect(getLatestTaskProjectMoveEntityIds([op])).toEqual(['taskT', 'sub1']); + }); + + it('returns undefined for a legacy LWW op with entityIds but no authenticated footprint (no laundering)', () => { + const op = lwwMoveOp({ entityIds: ['taskT', 'victim'] }); + + expect(getLatestTaskProjectMoveEntityIds([op])).toBeUndefined(); + }); + + it('takes the raw TASK_SHARED_UPDATE footprint ROOT from the authenticated payload, not the tampered entityId envelope (GHSA-8pxh-mgc7-gp3g)', () => { + // A raw TASK_SHARED_UPDATE op's entityId is NOT bound to payload.id by the + // decrypt gate (that gate only covers LWW ops). A compromised server + // retargets the plaintext envelope entityId to 'victim' while the + // authenticated payload still moves the real task 'taskT'. The footprint + // root must come from the authenticated payload.task.id. + const op = { + id: 'op-upd', + actionType: ActionType.TASK_SHARED_UPDATE, + opType: OpType.Update, + entityType: 'TASK', + entityId: 'victim', + payload: { + actionPayload: { + task: { id: 'taskT', changes: { projectId: 'proj-2' } }, + projectMoveSubTaskIds: ['sub1'], + }, + entityChanges: [], + }, + clientId: 'c', + vectorClock: { c: 1 }, + timestamp: 1, + schemaVersion: CURRENT_SCHEMA_VERSION, + } as unknown as Operation; + + const result = getLatestTaskProjectMoveEntityIds([op]); + expect(result).toEqual(['taskT', 'sub1']); + expect(result).not.toContain('victim'); + }); }); it('should ensure _convertToLWWUpdatesIfNeeded merged payload has id even when base entity lacks id', () => { diff --git a/src/app/op-log/sync/conflict-resolution.service.ts b/src/app/op-log/sync/conflict-resolution.service.ts index 775231c906..4af18fbbf2 100644 --- a/src/app/op-log/sync/conflict-resolution.service.ts +++ b/src/app/op-log/sync/conflict-resolution.service.ts @@ -190,12 +190,24 @@ const mergeMarkedProjectDeleteOps = (localOps: Operation[]): Operation | undefin }; const getTaskProjectMoveEntityIds = (operation: Operation): string[] | undefined => { - if ( - operation.actionType === toLwwUpdateActionType('TASK') && - operation.entityId && - Array.isArray(operation.entityIds) - ) { - return Array.from(new Set([operation.entityId, ...operation.entityIds])); + // Reuse a prior synthetic LWW op's footprint ONLY from the AUTHENTICATED + // payload (projectMoveFootprint), never the plaintext op.entityIds envelope. + // A compromised server can tamper a remote op's envelope; reading it here + // would launder those ids into a freshly-authenticated merged op that every + // client then trusts — the same GHSA-8pxh-mgc7-gp3g vector, one merge removed. + // Legacy LWW ops carry no authenticated footprint → no reusable set (the + // merged op then falls back to receiving-state repair, mirroring the reducers). + if (operation.actionType === toLwwUpdateActionType('TASK') && operation.entityId) { + const footprint = isLwwUpdatePayload(operation.payload) + ? operation.payload.projectMoveFootprint + : undefined; + if (!Array.isArray(footprint)) return undefined; + return Array.from( + new Set([ + operation.entityId, + ...footprint.filter((id): id is string => typeof id === 'string'), + ]), + ); } if ( @@ -215,11 +227,21 @@ const getTaskProjectMoveEntityIds = (operation: Operation): string[] | undefined const subTaskIds = actionPayload['projectMoveSubTaskIds']; if (!Array.isArray(subTaskIds)) return undefined; + // SECURITY: the footprint ROOT must come from the AUTHENTICATED payload + // (actionPayload.task.id), NOT the plaintext op.entityId envelope. Unlike LWW + // ops — whose entityId is bound to payload.id by assertDecryptedOpMetadataIntegrity + // — a raw TASK_SHARED_UPDATE op's entityId is unauthenticated, so reading it here + // would let a compromised server launder a victim id into the authenticated + // projectMoveFootprint of the synthesized merged op. GHSA-8pxh-mgc7-gp3g. + const task = actionPayload['task']; + const rootId = + task && typeof task === 'object' + ? (task as Record)['id'] + : undefined; + if (typeof rootId !== 'string') return undefined; + return Array.from( - new Set([ - operation.entityId, - ...subTaskIds.filter((id): id is string => typeof id === 'string'), - ]), + new Set([rootId, ...subTaskIds.filter((id): id is string => typeof id === 'string')]), ); }; @@ -433,10 +455,19 @@ export class ConflictResolutionService { const actionPayload = isSingletonEntityId(entityId) ? basePayload : { ...basePayload, id: entityId }; + // Compute the move footprint once and carry it BOTH in the plaintext + // envelope (op.entityIds — the server needs it for its indexed conflict + // detection and cannot read the encrypted payload) AND inside the + // authenticated payload (projectMoveFootprint). Remote clients trust only + // the authenticated copy, closing the envelope-injection vector + // (GHSA-8pxh-mgc7-gp3g). + const moveFootprint = + entityIds !== undefined ? Array.from(new Set([entityId, ...entityIds])) : undefined; const payload: LwwUpdatePayload = { actionPayload, entityChanges: [], lwwUpdateMode, + ...(moveFootprint !== undefined && { projectMoveFootprint: moveFootprint }), }; return { id: uuidv7(), @@ -444,9 +475,7 @@ export class ConflictResolutionService { opType: OpType.Update, entityType, entityId, - ...(entityIds !== undefined && { - entityIds: Array.from(new Set([entityId, ...entityIds])), - }), + ...(moveFootprint !== undefined && { entityIds: moveFootprint }), payload, clientId, vectorClock, diff --git a/src/app/op-log/sync/superseded-operation-resolver.service.spec.ts b/src/app/op-log/sync/superseded-operation-resolver.service.spec.ts index fb4365c841..3eddfba67a 100644 --- a/src/app/op-log/sync/superseded-operation-resolver.service.spec.ts +++ b/src/app/op-log/sync/superseded-operation-resolver.service.spec.ts @@ -5,7 +5,10 @@ import { OperationLogStoreService, } from '../persistence/operation-log-store.service'; import { VectorClockService } from './vector-clock.service'; -import { ConflictResolutionService } from './conflict-resolution.service'; +import { + ConflictResolutionService, + getLatestTaskProjectMoveEntityIds, +} from './conflict-resolution.service'; import { LockService } from './lock.service'; import { SnackService } from '../../core/snack/snack.service'; import { CLIENT_ID_PROVIDER } from '../util/client-id.provider'; @@ -403,7 +406,7 @@ describe('SupersededOperationResolverService', () => { expect(appendedOp.entityIds).toBeUndefined(); }); - it('should preserve a project-move footprint through another LWW replacement', async () => { + it('should preserve an authenticated project-move footprint through another LWW replacement', async () => { const supersededOp = createMockOperation( 'op-1', 'TASK', @@ -413,6 +416,13 @@ describe('SupersededOperationResolverService', () => { ); supersededOp.actionType = '[TASK] LWW Update' as ActionType; supersededOp.entityIds = ['task-1', 'subtask-1']; + // New-style synthetic LWW op: footprint lives in the authenticated payload. + supersededOp.payload = { + actionPayload: { id: 'task-1' }, + entityChanges: [], + lwwUpdateMode: 'replace', + projectMoveFootprint: ['task-1', 'subtask-1'], + } as unknown as Operation['payload']; mockConflictResolutionService.getCurrentEntityState.and.resolveTo({ id: 'task-1', title: 'Test Task', @@ -422,9 +432,31 @@ describe('SupersededOperationResolverService', () => { const appendedOp = mockOpLogStore.appendWithVectorClockUpdate.calls.first() .args[0] as Operation; + // The footprint is derived from the authenticated payload and passed to + // createLWWUpdateOp (which embeds it in both entityIds and the payload; + // real embedding is covered by the conflict-resolution spec). expect(appendedOp.entityIds).toEqual(['task-1', 'subtask-1']); }); + it('does not reuse a legacy LWW footprint that exists only in the plaintext entityIds envelope (GHSA-8pxh-mgc7-gp3g)', () => { + // A pre-fix synthetic LWW op carries entityIds but no authenticated + // projectMoveFootprint. It must NOT be reused as a footprint — reading the + // envelope here would launder a (potentially tampered) value into a freshly + // authenticated replacement op. The replacement falls back to + // receiving-state repair instead. + const legacyOp = createMockOperation( + 'op-legacy', + 'TASK', + 'task-1', + { clientA: 5 }, + 1000, + ); + legacyOp.actionType = '[TASK] LWW Update' as ActionType; + legacyOp.entityIds = ['task-1', 'subtask-1']; + + expect(getLatestTaskProjectMoveEntityIds([legacyOp])).toBeUndefined(); + }); + it('should create single merged op for multiple superseded ops on same entity', async () => { const supersededOp1 = createMockOperation( 'op-1', @@ -450,6 +482,7 @@ describe('SupersededOperationResolverService', () => { supersededOp1.actionType = ActionType.TASK_SHARED_UPDATE; supersededOp1.payload = { actionPayload: { + task: { id: 'task-1', changes: { projectId: 'proj-2' } }, projectMoveSubTaskIds: ['former-subtask'], }, entityChanges: [], @@ -457,6 +490,7 @@ describe('SupersededOperationResolverService', () => { supersededOp2.actionType = ActionType.TASK_SHARED_UPDATE; supersededOp2.payload = { actionPayload: { + task: { id: 'task-1', changes: { projectId: 'proj-2' } }, projectMoveSubTaskIds: ['current-subtask'], }, entityChanges: [], diff --git a/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.spec.ts b/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.spec.ts index a5b9b7d2c0..178c527f03 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.spec.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.spec.ts @@ -1985,7 +1985,11 @@ describe('lwwUpdateMetaReducer', () => { ); }); - it('should replay only the LWW operation footprint on divergent state', () => { + it('should replay only the authenticated move footprint on divergent state', () => { + // The move footprint is the authenticated meta.projectMoveFootprint (sourced from + // the encrypted payload), NOT the plaintext meta.entityIds envelope. + // 'receiver-child' is a divergent receiver-only child outside the footprint, + // so it must NOT be relocated. const state = createStateWithProjects( PROJECT_A, [TASK_ID, 'captured-child', 'receiver-child'], @@ -2020,7 +2024,7 @@ describe('lwwUpdateMetaReducer', () => { isPersistent: true, entityType: 'TASK', entityId: TASK_ID, - entityIds: [TASK_ID, 'captured-child'], + projectMoveFootprint: [TASK_ID, 'captured-child'], }, }; @@ -2041,6 +2045,101 @@ describe('lwwUpdateMetaReducer', () => { ]); }); + it('ignores a tampered meta.entityIds envelope and does not relocate an unrelated task (GHSA-8pxh-mgc7-gp3g)', () => { + // A compromised sync server appends an unrelated 'victim' task to the + // plaintext meta.entityIds envelope of a genuine, correctly-encrypted move + // op. 'victim' is a root task with no parent relationship to TASK_ID, so + // nothing authentic implicates it in the move — it must stay in PROJECT_A. + const state = createStateWithProjects(PROJECT_A, [TASK_ID, 'victim'], []); + state[TASK_FEATURE_NAME] = { + ...state[TASK_FEATURE_NAME]!, + ids: [TASK_ID, 'victim'], + entities: { + [TASK_ID]: createMockTask({ projectId: PROJECT_A, subTaskIds: [] }), + ['victim']: createMockTask({ id: 'victim', projectId: PROJECT_A }), + }, + }; + const action = { + type: '[TASK] LWW Update', + id: TASK_ID, + projectId: PROJECT_B, + title: 'Moved task', + meta: { + isPersistent: true, + entityType: 'TASK', + entityId: TASK_ID, + // Attacker-injected: 'victim' is not part of any authenticated footprint. + entityIds: [TASK_ID, 'victim'], + }, + }; + + reducer(state, action); + + const updatedState = mockReducer.calls.mostRecent().args[0] as Partial; + expect(updatedState[TASK_FEATURE_NAME]?.entities['victim']?.projectId).toBe( + PROJECT_A, + ); + expect(updatedState[PROJECT_FEATURE_NAME]?.entities[PROJECT_A]?.taskIds).toContain( + 'victim', + ); + expect( + updatedState[PROJECT_FEATURE_NAME]?.entities[PROJECT_B]?.taskIds, + ).not.toContain('victim'); + }); + + it('relocates only the authenticated footprint when meta.entityIds is tampered alongside it (GHSA-8pxh-mgc7-gp3g)', () => { + // Production tamper shape: a genuine authenticated footprint + // (meta.projectMoveFootprint) plus a larger meta.entityIds envelope into which the + // server injected an unrelated 'victim'. Only the footprint members move. + const state = createStateWithProjects( + PROJECT_A, + [TASK_ID, 'captured-child', 'victim'], + [], + ); + state[TASK_FEATURE_NAME] = { + ...state[TASK_FEATURE_NAME]!, + ids: [TASK_ID, 'captured-child', 'victim'], + entities: { + [TASK_ID]: createMockTask({ + projectId: PROJECT_A, + subTaskIds: ['captured-child'], + }), + ['captured-child']: createMockTask({ + id: 'captured-child', + parentId: TASK_ID, + projectId: PROJECT_A, + }), + ['victim']: createMockTask({ id: 'victim', projectId: PROJECT_A }), + }, + }; + const action = { + type: '[TASK] LWW Update', + id: TASK_ID, + projectId: PROJECT_B, + title: 'Moved task', + meta: { + isPersistent: true, + entityType: 'TASK', + entityId: TASK_ID, + projectMoveFootprint: [TASK_ID, 'captured-child'], + entityIds: [TASK_ID, 'captured-child', 'victim'], + }, + }; + + reducer(state, action); + + const updatedState = mockReducer.calls.mostRecent().args[0] as Partial; + expect(updatedState[TASK_FEATURE_NAME]?.entities['captured-child']?.projectId).toBe( + PROJECT_B, + ); + expect(updatedState[TASK_FEATURE_NAME]?.entities['victim']?.projectId).toBe( + PROJECT_A, + ); + expect(updatedState[PROJECT_FEATURE_NAME]?.entities[PROJECT_A]?.taskIds).toContain( + 'victim', + ); + }); + it('should repair stale project references without changing target backlog placement', () => { const state = createStateWithProjects(PROJECT_A, [], [TASK_ID]); state[PROJECT_FEATURE_NAME]!.entities[PROJECT_A] = createMockProject({ diff --git a/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.ts b/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.ts index 455c4a5008..d6b4703847 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.ts @@ -28,7 +28,11 @@ import { filterTaskIdArraysFromTagOrProjectPayload } from '../../../op-log/apply import { appStateFeatureKey } from '../../app-state/app-state.reducer'; import { getDbDateStr, isDBDateStr } from '../../../util/get-db-date-str'; import { isTodayWithOffset } from '../../../util/is-today.util'; -import { getProjectOrUndefined, repairTaskProjectForLww } from './task-shared-helpers'; +import { + getProjectOrUndefined, + parseMoveFootprint, + repairTaskProjectForLww, +} from './task-shared-helpers'; import { withLocalOnlySyncSettings } from '../../../features/config/local-only-sync-settings.util'; import { SyncConfig } from '../../../features/config/global-config.model'; import { LwwUpdateMode } from '../../../op-log/core/operation.types'; @@ -489,6 +493,7 @@ export const lwwUpdateMetaReducer: MetaReducer = ( lwwUpdateMode?: LwwUpdateMode; isApplyingFromOtherClient?: boolean; recreatesEntityAfterDelete?: boolean; + projectMoveFootprint?: readonly string[]; } | undefined; let entityData: Record = {}; @@ -842,25 +847,21 @@ export const lwwUpdateMetaReducer: MetaReducer = ( } } - const meta = actionAny['meta']; - const explicitEntityIds = - meta && - typeof meta === 'object' && - Array.isArray((meta as { entityIds?: unknown }).entityIds) - ? (meta as { entityIds: unknown[] }).entityIds.filter( - (id): id is string => typeof id === 'string', - ) - : undefined; + // Use the AUTHENTICATED move footprint (from the encrypted payload, + // surfaced onto meta.projectMoveFootprint), NOT the plaintext, server-tamperable + // meta.entityIds envelope — otherwise a compromised sync server could + // relocate arbitrary tasks (GHSA-8pxh-mgc7-gp3g). + const authFootprint = parseMoveFootprint(actionMeta?.projectMoveFootprint); if (!newIsSubTask) { // Root snapshots repair every project list, even when projectId is - // unchanged. New synthetic LWW ops replay their source footprint; - // old ops without entityIds retain receiving-state repair behavior. + // unchanged. New synthetic LWW ops replay their authenticated footprint; + // old ops without a footprint retain receiving-state repair behavior. updatedState = repairTaskProjectForLww( updatedState, updatedEntity as unknown as Task, newProjectId, - explicitEntityIds, + authFootprint, ); } else { updatedState = syncProjectTaskIds( diff --git a/src/app/root-store/meta/task-shared-meta-reducers/section-shared.reducer.spec.ts b/src/app/root-store/meta/task-shared-meta-reducers/section-shared.reducer.spec.ts index 70a08c3070..51023d9df2 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/section-shared.reducer.spec.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/section-shared.reducer.spec.ts @@ -651,7 +651,11 @@ describe('sectionSharedMetaReducer', () => { expect(updatedState[SECTION_FEATURE_NAME].entities['sOther']?.taskIds).toEqual([]); }); - it('strips old-project sections when a task moves through LWW resolution', () => { + it('strips old-project sections for the authenticated footprint on LWW resolution', () => { + // Footprint comes from the authenticated meta.projectMoveFootprint (sourced from + // the encrypted payload), not the plaintext meta.entityIds envelope. + // 'receiverOnly' is a divergent receiver-side child outside the footprint, + // so it must survive. const state = stateWith( { parent: { projectId: 'oldP', subTaskIds: [] }, @@ -674,7 +678,7 @@ describe('sectionSharedMetaReducer', () => { type: '[TASK] LWW Update', id: 'parent', projectId: 'newP', - meta: { entityIds: ['parent', 'sub1'] }, + meta: { projectMoveFootprint: ['parent', 'sub1'] }, } as Action); const updated = ( @@ -685,6 +689,46 @@ describe('sectionSharedMetaReducer', () => { expect(updated.entities['sOld']?.taskIds).toEqual(['receiverOnly']); }); + it('ignores a tampered meta.entityIds envelope on LWW resolution (GHSA-8pxh-mgc7-gp3g)', () => { + // A compromised server injects 'victim' into the plaintext meta.entityIds + // envelope. The reducer derives the footprint only from the authenticated + // meta.projectMoveFootprint, so 'victim' is not stripped from its section. + const state = stateWith( + { + parent: { projectId: 'oldP', subTaskIds: ['sub1'] }, + sub1: { projectId: 'oldP', parentId: 'parent' }, + victim: { projectId: 'oldP' }, + }, + [ + { + id: 'sOld', + contextId: 'oldP', + contextType: WorkContextType.PROJECT, + title: 'old project section', + taskIds: ['parent', 'sub1', 'victim'], + }, + ], + ); + addProject(state, 'newP'); + + metaReducer(state, { + type: '[TASK] LWW Update', + id: 'parent', + projectId: 'newP', + meta: { + projectMoveFootprint: ['parent', 'sub1'], + entityIds: ['parent', 'sub1', 'victim'], + }, + } as Action); + + const updated = ( + mockReducer.calls.mostRecent().args[0] as RootState & { + [SECTION_FEATURE_NAME]: SectionState; + } + )[SECTION_FEATURE_NAME]; + expect(updated.entities['sOld']?.taskIds).toContain('victim'); + }); + it('repairs stale other-project sections for a same-project LWW snapshot', () => { const state = stateWith({ parent: { projectId: 'project1' } }, [ { diff --git a/src/app/root-store/meta/task-shared-meta-reducers/section-shared.reducer.ts b/src/app/root-store/meta/task-shared-meta-reducers/section-shared.reducer.ts index 95d07f2a51..b8f4753638 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/section-shared.reducer.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/section-shared.reducer.ts @@ -24,6 +24,7 @@ import { enrichDeleteProjectAction, getProjectOrUndefined, isValidTaskProjectIdUpdate, + parseMoveFootprint, } from './task-shared-helpers'; import { toLwwUpdateActionType } from '../../../op-log/core/lww-update-action-types'; @@ -434,7 +435,7 @@ const ACTION_HANDLERS: Record = { id?: unknown; parentId?: unknown; projectId?: unknown; - meta?: { entityIds?: unknown }; + meta?: { projectMoveFootprint?: readonly string[] }; }; if (typeof update.id !== 'string') return state; @@ -443,12 +444,14 @@ const ACTION_HANDLERS: Record = { | undefined; const currentTask = currentTaskCandidate?.id === update.id ? currentTaskCandidate : undefined; - const explicitEntityIds = Array.isArray(update.meta?.entityIds) - ? update.meta.entityIds.filter((id): id is string => typeof id === 'string') - : undefined; + // Use the AUTHENTICATED move footprint (meta.projectMoveFootprint from the encrypted + // payload), never the plaintext meta.entityIds envelope. Mirrors + // repairTaskProjectForLww so the two LWW-TASK trust sites cannot drift. + // GHSA-8pxh-mgc7-gp3g. + const authFootprint = parseMoveFootprint(update.meta?.projectMoveFootprint); const affectedTaskIds = - explicitEntityIds !== undefined - ? Array.from(new Set([update.id, ...explicitEntityIds])) + authFootprint !== undefined + ? Array.from(new Set([update.id, ...authFootprint])) : collectTaskAndSubTaskIds(state, [update.id]); const hasParentId = Object.prototype.hasOwnProperty.call(update, 'parentId'); diff --git a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts index 1183555c9d..1a0ce64ca8 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts @@ -172,11 +172,29 @@ export const removeTasksFromAllProjects = ( }; }; +/** + * Parses the authenticated LWW move footprint surfaced onto an action's `meta` + * (`meta.projectMoveFootprint`, sourced from the E2EE-encrypted `payload + * .projectMoveFootprint`). Returns `undefined` for legacy/non-move ops that + * carry no authenticated footprint, so callers fall back to receiving-state + * repair. Both LWW-TASK trust sites (project repair + section membership) MUST + * use this — never the plaintext `meta.entityIds` envelope, which a compromised + * sync server can tamper with. GHSA-8pxh-mgc7-gp3g. + */ +export const parseMoveFootprint = (moveFootprint: unknown): string[] | undefined => { + if (!Array.isArray(moveFootprint)) return undefined; + const ids = moveFootprint.filter((id): id is string => typeof id === 'string'); + // Empty means "no usable footprint" → fall back to receiving-state repair, + // never "relocate the root task alone" (which an empty array would imply). + return ids.length > 0 ? ids : undefined; +}; + /** * Repairs a root task's project relationship after an LWW update. New LWW - * operations derived from project moves carry the source operation's entityIds, - * which are the deterministic move footprint. Legacy LWW operations have no - * such footprint and fall back to deriving children from receiving state. + * operations derived from project moves carry an authenticated move footprint + * (`meta.projectMoveFootprint`), which is the deterministic set of tasks to relocate. + * Legacy LWW operations have no such footprint and fall back to deriving + * children from receiving state. * * Every project is scanned so stale one-sided references are removed. The * destination project's existing root placement (regular vs backlog) and