diff --git a/packages/core/src/api/index.ts b/packages/core/src/api/index.ts index 86adc4cde..10c2585bb 100644 --- a/packages/core/src/api/index.ts +++ b/packages/core/src/api/index.ts @@ -85,6 +85,7 @@ import { LazyPromise } from "../utils/lazy-promise.js"; import { InboxApiKeys } from "./inbox-api-keys.js"; import { Circle } from "./circle.js"; import { Wrapped } from "./wrapped.js"; +import { FailedSyncItems } from "../collections/failed-sync-items.js"; type EventSourceConstructor = new ( uri: string, @@ -230,6 +231,7 @@ class Database { inboxApiKeys = new InboxApiKeys(this, this.tokenManager); inboxItemsHistory = new InboxItemsHistory(this); + failedSyncItems = new FailedSyncItems(this); wrapped = new Wrapped(this); @@ -353,6 +355,7 @@ class Database { await this.monographsCollection.init(); await this.inboxItemsHistory.init(); + await this.failedSyncItems.init(); await this.trash.init(); diff --git a/packages/core/src/api/sync/__tests__/process-chunk.test.js b/packages/core/src/api/sync/__tests__/process-chunk.test.js new file mode 100644 index 000000000..4338f5d66 --- /dev/null +++ b/packages/core/src/api/sync/__tests__/process-chunk.test.js @@ -0,0 +1,289 @@ +/* +This file is part of the Notesnook project (https://notesnook.com/) + +Copyright (C) 2023 Streetwriters (Private) Limited + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*/ + +import { + databaseTest, + loginFakeUser, + TEST_NOTE +} from "../../../../__tests__/utils/index.ts"; +import { KEY_VERSION } from "../types.ts"; +import { test, expect, vi } from "vitest"; + +const CURRENT_DATABASE_VERSION = 6.1; + +function chunkItem(id, keyVersion) { + return { + id, + v: CURRENT_DATABASE_VERSION, + keyVersion, + format: "base64", + alg: "xchacha20-poly1305", + cipher: "ciphertext", + iv: "iv", + salt: "salt", + length: 1 + }; +} + +function decryptFailure() { + return Promise.reject( + new Error("ciphertext cannot be decrypted using that key") + ); +} + +function failedItemPayload(itemId, itemType) { + return { + itemId, + itemType, + cipher: chunkItem(itemId, KEY_VERSION.LEGACY), + errors: [`Failed to decrypt item ${itemId} with key version 0.`], + dateSynced: Date.now() + }; +} + +test("items with a key version that has no matching key are stored in failedSyncItems", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const item = chunkItem("setting-1", KEY_VERSION.DEK); + const keys = [{ version: KEY_VERSION.LEGACY, key: { key: "legacy-key" } }]; + + // decryptMulti is never called for the DEK version group (no matching key), + // so only the per-item retry path runs. Make all retries fail. + vi.spyOn(db.storage(), "decrypt").mockImplementation(decryptFailure); + + await db.syncer.sync.processChunk( + { type: "settingitem", count: 1, items: [item] }, + keys, + { type: "fetch" } + ); + + const failedItems = await db.failedSyncItems.all.items(); + expect(failedItems).toHaveLength(1); + expect(failedItems[0].itemId).toBe(item.id); + expect(failedItems[0].itemType).toBe("settingitem"); + expect(failedItems[0].cipher.id).toBe(item.id); + expect(failedItems[0].errors.length).toBe(keys.length); + expect(failedItems[0].errors[0]).toContain( + "ciphertext cannot be decrypted" + ); + })); + +test("items that fail to decrypt with all available keys are stored in failedSyncItems", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const item = chunkItem("setting-1", KEY_VERSION.LEGACY); + const keys = [ + { version: KEY_VERSION.LEGACY, key: { key: "legacy-key" } }, + { version: KEY_VERSION.DEK, key: { key: "dek-key" } } + ]; + + vi.spyOn(db.storage(), "decryptMulti").mockImplementation(decryptFailure); + vi.spyOn(db.storage(), "decrypt").mockImplementation(decryptFailure); + + await db.syncer.sync.processChunk( + { type: "settingitem", count: 1, items: [item] }, + keys, + { type: "fetch" } + ); + + const failedItems = await db.failedSyncItems.all.items(); + expect(failedItems).toHaveLength(1); + expect(failedItems[0].errors.length).toBe(keys.length); + expect(failedItems[0].errors[0]).toContain("key version 0"); + expect(failedItems[0].errors[1]).toContain("key version 1"); + })); + +test("items that fail batch decryption but succeed on a fallback key are decrypted and not stored as failed", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const item = chunkItem("setting-1", KEY_VERSION.DEK); + const keys = [ + { version: KEY_VERSION.LEGACY, key: { key: "legacy-key" } }, + { version: KEY_VERSION.DEK, key: { key: "dek-key" } } + ]; + + // Batch decryption with the DEK key fails... + vi.spyOn(db.storage(), "decryptMulti").mockImplementation(decryptFailure); + // ...but the per-item retry succeeds with the legacy key. + const decryptedItem = JSON.stringify({ + id: item.id, + type: "settingitem", + deleted: true, + dateCreated: Date.now(), + dateModified: Date.now() + }); + vi.spyOn(db.storage(), "decrypt").mockImplementation((key) => + key === keys[0].key ? Promise.resolve(decryptedItem) : decryptFailure() + ); + + await db.syncer.sync.processChunk( + { type: "settingitem", count: 1, items: [item] }, + keys, + { type: "fetch" } + ); + + const failedItems = await db.failedSyncItems.all.items(); + expect(failedItems).toHaveLength(0); + expect(db.storage().decrypt).toHaveBeenCalledTimes(1); + })); + +test("successfully decrypted items are not stored in failedSyncItems", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const item = chunkItem("setting-1", KEY_VERSION.LEGACY); + const keys = [{ version: KEY_VERSION.LEGACY, key: { key: "legacy-key" } }]; + + const decryptedItem = JSON.stringify({ + id: item.id, + type: "settingitem", + deleted: true, + dateCreated: Date.now(), + dateModified: Date.now() + }); + vi.spyOn(db.storage(), "decryptMulti").mockResolvedValue([decryptedItem]); + + await db.syncer.sync.processChunk( + { type: "settingitem", count: 1, items: [item] }, + keys, + { type: "fetch" } + ); + + const failedItems = await db.failedSyncItems.all.items(); + expect(failedItems).toHaveLength(0); + })); + +test("failedSyncItems.add stores an item and assigns it a new id", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const itemId = await db.notes.add(TEST_NOTE); + const id = await db.failedSyncItems.add(failedItemPayload(itemId, "note")); + + expect(id).toBeDefined(); + expect(id).not.toBe(itemId); + + const items = await db.failedSyncItems.all.items(); + expect(items).toHaveLength(1); + expect(items[0].itemId).toBe(itemId); + expect(items[0].itemType).toBe("note"); + expect(items[0].cipher.id).toBe(itemId); + expect(items[0].errors).toEqual([ + `Failed to decrypt item ${itemId} with key version 0.` + ]); + })); + +test("failedSyncItems.delete removes the failed item and soft-deletes the underlying item", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const noteId = await db.notes.add(TEST_NOTE); + const failedId = await db.failedSyncItems.add( + failedItemPayload(noteId, "note") + ); + + await db.failedSyncItems.delete([failedId]); + + const failedItems = await db.failedSyncItems.all.items(); + expect(failedItems).toHaveLength(0); + expect(await isItemDeleted(db, "notes", noteId)).toBe(true); + })); + +test("failedSyncItems.delete groups items by type and soft-deletes in the right collections", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const noteId = await db.notes.add(TEST_NOTE); + const tagId = await db.tags.add({ title: "test-tag" }); + const failedNoteId = await db.failedSyncItems.add( + failedItemPayload(noteId, "note") + ); + const failedTagId = await db.failedSyncItems.add( + failedItemPayload(tagId, "tag") + ); + + await db.failedSyncItems.delete([failedNoteId, failedTagId]); + + expect(await db.failedSyncItems.all.items()).toHaveLength(0); + expect(await isItemDeleted(db, "notes", noteId)).toBe(true); + expect(await isItemDeleted(db, "tags", tagId)).toBe(true); + })); + +test("failedSyncItems.delete only deletes the requested ids", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const noteId1 = await db.notes.add(TEST_NOTE); + const noteId2 = await db.notes.add(TEST_NOTE); + const failedId1 = await db.failedSyncItems.add( + failedItemPayload(noteId1, "note") + ); + await db.failedSyncItems.add(failedItemPayload(noteId2, "note")); + + await db.failedSyncItems.delete([failedId1]); + + const failedItems = await db.failedSyncItems.all.items(); + expect(failedItems).toHaveLength(1); + expect(failedItems[0].itemId).toBe(noteId2); + expect(await isItemDeleted(db, "notes", noteId1)).toBe(true); + expect(await db.notes.collection.get(noteId2)).toBeDefined(); + })); + +test("failedSyncItems.delete skips unknown collection types gracefully", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const failedId = await db.failedSyncItems.add( + failedItemPayload("unknown-id", "unknown-type") + ); + + // Unknown type has no entry in SYNC_COLLECTIONS_MAP so it's skipped + // gracefully; the failed item row itself is still removed. + await expect(db.failedSyncItems.delete([failedId])).resolves.not.toThrow(); + expect(await db.failedSyncItems.all.items()).toHaveLength(0); + })); + +test("failedSyncItems.clear removes all failed items", () => + databaseTest().then(async (db) => { + await loginFakeUser(db); + + const noteId1 = await db.notes.add(TEST_NOTE); + const noteId2 = await db.notes.add(TEST_NOTE); + await db.failedSyncItems.add(failedItemPayload(noteId1, "note")); + await db.failedSyncItems.add(failedItemPayload(noteId2, "note")); + + await db.failedSyncItems.clear(); + + expect(await db.failedSyncItems.all.items()).toHaveLength(0); + expect(await isItemDeleted(db, "notes", noteId1)).toBeTruthy(); + expect(await isItemDeleted(db, "notes", noteId2)).toBeTruthy(); + })); + +async function isItemDeleted(db, collectionName, itemId) { + const item = await db + .sql() + .selectFrom(collectionName) + .selectAll() + .where("id", "==", itemId) + .executeTakeFirst(); + return item?.deleted === true; +} diff --git a/packages/core/src/api/sync/index.ts b/packages/core/src/api/sync/index.ts index 30c95320e..573395b4a 100644 --- a/packages/core/src/api/sync/index.ts +++ b/packages/core/src/api/sync/index.ts @@ -51,6 +51,7 @@ import { SYNC_COLLECTIONS_MAP, SyncableItemType, SyncInboxItem, + SyncItem, SyncTransferItem } from "./types.js"; import { DownloadableFile } from "../../database/fs.js"; @@ -373,7 +374,7 @@ export class Sync { const versionMap = new Map(); for (const item of chunk.items) { - const keyVersion = item.keyVersion ?? KEY_VERSION.UNKNOWN; + const keyVersion = item.keyVersion ?? KEY_VERSION.LEGACY; const group = itemsByKeyVersion.get(keyVersion); if (group) { group.push(item); @@ -383,48 +384,76 @@ export class Sync { versionMap.set(item.id, item.v); } - const unknownKeyVersionItems = itemsByKeyVersion.get(KEY_VERSION.UNKNOWN); - if ( - unknownKeyVersionItems && - unknownKeyVersionItems.length > 0 && - keys.length > 1 - ) { - this.logger.info( - `Decrypting ${unknownKeyVersionItems.length} items with unknown key version using all available keys.` - ); - for (const item of unknownKeyVersionItems ?? []) { - for (const keyInfo of keys) { - try { - this.logger.info("Decrypting unknown key version item using key", { - keyInfo: keyInfo.version - }); - const decryptedItem = await this.db - .storage() - .decrypt(keyInfo.key, item); - decrypted.push(decryptedItem); - break; - } catch (error) { - this.logger.error( - new Error( - `Failed to decrypt item ${item.id} with key version ${keyInfo.version}.` - ) - ); - } - } + const failedItems: SyncItem[] = []; + for (const [keyVersion, items] of itemsByKeyVersion.entries()) { + const keyInfo = keys.find((k) => k.version === keyVersion); + if (!keyInfo) { + this.logger.error( + new Error( + `No key found for key version ${keyVersion}. Retrying items will all available keys.` + ) + ); + failedItems.push(...items); + continue; } - } - - for (const keyInfo of keys) { - const itemsToDecrypt = itemsByKeyVersion.get(keyInfo.version); - if (!itemsToDecrypt || itemsToDecrypt.length === 0) continue; this.logger.info("Decrypting using key", { keyInfo: keyInfo.version, - items: itemsToDecrypt.length + items: items.length }); - decrypted.push( - ...(await this.db.storage().decryptMulti(keyInfo.key, itemsToDecrypt)) - ); + try { + const decryptedItems = await this.db + .storage() + .decryptMulti(keyInfo.key, items); + decrypted.push(...decryptedItems); + } catch (error) { + this.logger.error( + error, + `Failed to decrypt items with key version ${keyInfo.version}.` + ); + failedItems.push(...items); + } + } + + if (failedItems.length > 0) { + for (const item of failedItems) { + const decryptionErrors: string[] = []; + for (const keyInfo of keys) { + this.logger.info("Decrypting failed sync item using key", { + keyInfo: keyInfo.version + }); + const decryptedItem = await this.db + .storage() + .decrypt(keyInfo.key, item) + .catch((error) => { + decryptionErrors.push( + `Failed to decrypt item ${item.id} with key version ${ + keyInfo.version + }. Error: ${ + error instanceof Error ? error.message : JSON.stringify(error) + }` + ); + this.logger.error( + error, + `Failed to decrypt item ${item.id} with key version ${keyInfo.version}.` + ); + return false; + }); + if (typeof decryptedItem === "string") { + decrypted.push(decryptedItem); + break; + } + } + if (decryptionErrors.length === keys.length) { + await this.db.failedSyncItems.add({ + itemId: item.id, + itemType: itemType, + cipher: item, + errors: decryptionErrors, + dateSynced: Date.now() + }); + } + } } const deserialized: MaybeDeletedItem[] = []; diff --git a/packages/core/src/api/sync/types.ts b/packages/core/src/api/sync/types.ts index 6c7be745d..e8dde136f 100644 --- a/packages/core/src/api/sync/types.ts +++ b/packages/core/src/api/sync/types.ts @@ -21,8 +21,7 @@ import { Cipher } from "@notesnook/crypto"; export const KEY_VERSION = { LEGACY: 0, - DEK: 1, - UNKNOWN: -1 + DEK: 1 } as const; export type KeyVersion = (typeof KEY_VERSION)[keyof typeof KEY_VERSION]; diff --git a/packages/core/src/collections/failed-sync-items.ts b/packages/core/src/collections/failed-sync-items.ts new file mode 100644 index 000000000..6011d596f --- /dev/null +++ b/packages/core/src/collections/failed-sync-items.ts @@ -0,0 +1,94 @@ +/* +This file is part of the Notesnook project (https://notesnook.com/) + +Copyright (C) 2023 Streetwriters (Private) Limited + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*/ + +import { BaseItem, FailedSyncItem } from "../types.js"; +import Database from "../api/index.js"; +import { ICollection } from "./collection.js"; +import { SQLCollection } from "../database/sql-collection.js"; +import { isFalse } from "../database/index.js"; +import { getId } from "../utils/id.js"; +import { SYNC_COLLECTIONS_MAP } from "../api/sync/types.js"; +import type { SyncableItemType } from "../api/sync/types.js"; +import { logger } from "../logger.js"; + +export class FailedSyncItems implements ICollection { + name = "failedsyncitems"; + readonly collection: SQLCollection<"failedsyncitems", FailedSyncItem>; + constructor(private readonly db: Database) { + this.collection = new SQLCollection( + db.sql, + db.transaction, + "failedsyncitems", + db.eventManager, + db.sanitizer + ); + } + + init() { + return this.collection.init(); + } + + async add(item: Omit>) { + const now = Date.now(); + const id = getId(); + await this.collection.upsert({ + type: "failedsyncitem", + dateCreated: now, + dateModified: now, + ...item, + id + }); + return id; + } + + get all() { + return this.collection.createFilter( + (qb) => qb.where(isFalse("deleted")), + this.db.options?.batchSize + ); + } + + async delete(ids: string[]) { + const groupedItems: Partial> = + {}; + for (const item of await this.all.items(ids)) { + const grouped = groupedItems[item.itemType] ?? []; + grouped.push(item); + groupedItems[item.itemType] = grouped; + } + for (const [itemType, items] of Object.entries(groupedItems)) { + const collectionType = SYNC_COLLECTIONS_MAP[itemType as SyncableItemType]; + const collection = this.db[collectionType]; + if (!collectionType || !collection) { + logger.error( + new Error(`Unknown collection type: ${itemType}`), + `Failed to delete failed sync items because the collection type is unknown.` + ); + continue; + } + await collection.collection.softDelete(items.map((item) => item.itemId)); + } + await this.collection.delete(ids); + } + + async clear() { + const ids = await this.all.ids(); + await this.delete(ids); + } +} diff --git a/packages/core/src/database/index.ts b/packages/core/src/database/index.ts index 550bc2bab..d4a7c9a3d 100644 --- a/packages/core/src/database/index.ts +++ b/packages/core/src/database/index.ts @@ -41,6 +41,7 @@ import { Attachment, Color, ContentItem, + FailedSyncItem, HistorySession, InboxItemHistory, ItemReference, @@ -94,6 +95,7 @@ export interface DatabaseSchema { vaults: SQLiteItem; monographs: SQLiteItem; inboxitemshistory: SQLiteItem; + failedsyncitems: SQLiteItem; } export type RawDatabaseSchema = DatabaseSchema & { @@ -279,6 +281,10 @@ const DataMappers: Partial void>> = { }, trash: (row) => { if (row.expiryDate) row.expiryDate = JSON.parse(row.expiryDate); + }, + failedsyncitem: (row) => { + if (row.cipher) row.cipher = JSON.parse(row.cipher); + if (row.errors) row.errors = JSON.parse(row.errors); } }; diff --git a/packages/core/src/database/migrations.ts b/packages/core/src/database/migrations.ts index 826bfcc98..87ce0be19 100644 --- a/packages/core/src/database/migrations.ts +++ b/packages/core/src/database/migrations.ts @@ -496,6 +496,21 @@ export class NNMigrationProvider implements MigrationProvider { .addColumn("errorContext", "text") .execute(); } + }, + "a-2026-08-10": { + async up(db) { + await db.schema + .createTable("failedsyncitems") + .ifNotExists() + .modifyEnd(sql`without rowid`) + .$call(addBaseColumns) + .addColumn("dateSynced", "integer") + .addColumn("cipher", "text") + .addColumn("errors", "text") + .addColumn("itemType", "text") + .addColumn("itemId", "text") + .execute(); + } } }; } diff --git a/packages/core/src/database/sql-collection.ts b/packages/core/src/database/sql-collection.ts index 4c3e11da1..a18f151e6 100644 --- a/packages/core/src/database/sql-collection.ts +++ b/packages/core/src/database/sql-collection.ts @@ -703,7 +703,8 @@ const VALID_SORT_OPTIONS: Record< shortcuts: [], vaults: [], monographs: [], - inboxitemshistory: [] + inboxitemshistory: [], + failedsyncitems: [] }; function sanitizeSortOptions(type: keyof DatabaseSchema, options: SortOptions) { diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 4cf8f2903..4c8b3bb71 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -19,6 +19,7 @@ along with this program. If not, see . import { Cipher } from "@notesnook/crypto"; import { isCipher } from "./utils/index.js"; +import type { SyncableItemType, SyncItem } from "./api/sync/types.js"; export type TimeFormat = "12-hour" | "24-hour"; export type DayFormat = "short" | "long"; @@ -85,6 +86,7 @@ export type Collections = { vaults: "vault"; monographs: "monograph"; inboxitemshistory: "inboxitemhistory"; + failedsyncitems: "failedsyncitem"; /** * @deprecated only kept here for migration purposes @@ -117,6 +119,7 @@ export type GroupableItem = ValueOf< | "vault" | "monograph" | "inboxitemhistory" + | "failedsyncitem" > >; @@ -140,6 +143,7 @@ export type ItemMap = { searchResult: HighlightedResult; monograph: Monograph; inboxitemhistory: InboxItemHistory; + failedsyncitem: FailedSyncItem; /** * @deprecated only kept here for migration purposes @@ -547,6 +551,14 @@ export interface InboxItemHistory extends BaseItem<"inboxitemhistory"> { errorContext?: string; } +export interface FailedSyncItem extends BaseItem<"failedsyncitem"> { + itemType: SyncableItemType; + itemId: string; + dateSynced: number; + cipher: SyncItem; + errors: string[]; +} + export type Match = { prefix: string; match: string;