fix(server): reject ambiguous KOReader file hashes

Treat partial-hash matches across distinct books as unresolved so progress cannot be written to an unrelated file. Preserve same-book duplicates and user-scoped manual links as valid disambiguation paths.

Fixes #1135

server/src/modules/koreader/koreader.repository.test.ts

@@ -20,8 +20,10 @@ function makeQueryChain(result: unknown) {
 }
 
 function makeDb() {
+  const select = vi.fn();
   return {
-    select: vi.fn(),
+    select,
+    selectDistinctOn: vi.fn((_on, fields) => select(fields)),
     insert: vi.fn(),
     update: vi.fn(),
     delete: vi.fn(),
@@ -114,6 +116,42 @@ describe('KoreaderRepository', () => {
       expect(db.select).toHaveBeenCalledTimes(1);
     });
 
+    it('returns the oldest file when current hash matches stay within one book', async () => {
+      const firstFile = { id: 10, bookId: 20, libraryId: 1, format: 'epub' };
+      const secondFile = { id: 11, bookId: 20, libraryId: 1, format: 'epub' };
+      db.select.mockReturnValue(makeQueryChain([firstFile, secondFile]));
+
+      await expect(repo.resolveBookFileByHash('abc123', null)).resolves.toEqual(firstFile);
+      expect(db.select).toHaveBeenCalledTimes(1);
+    });
+
+    it('returns null when a current hash matches different books', async () => {
+      db.select.mockReturnValue(
+        makeQueryChain([
+          { id: 10, bookId: 20, libraryId: 1, format: 'epub' },
+          { id: 11, bookId: 21, libraryId: 1, format: 'epub' },
+        ]),
+      );
+
+      await expect(repo.resolveBookFileByHash('abc123', null)).resolves.toBeNull();
+      expect(db.select).toHaveBeenCalledTimes(1);
+    });
+
+    it('uses a user-scoped manual link when a current hash matches different books', async () => {
+      const manualFile = { id: 11, bookId: 21, libraryId: 1, format: 'epub' };
+      db.select
+        .mockReturnValueOnce(
+          makeQueryChain([
+            { id: 10, bookId: 20, libraryId: 1, format: 'epub' },
+            { id: 11, bookId: 21, libraryId: 1, format: 'epub' },
+          ]),
+        )
+        .mockReturnValueOnce(makeQueryChain([manualFile]));
+
+      await expect(repo.resolveBookFileByHash('abc123', null, 7)).resolves.toEqual(manualFile);
+      expect(db.select).toHaveBeenCalledTimes(2);
+    });
+
     it('falls back to hash history when current hash lookup returns nothing', async () => {
       const file = { id: 10, bookId: 20, libraryId: 1, format: 'pdf' };
       db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([file]));
@@ -124,6 +162,18 @@ describe('KoreaderRepository', () => {
       expect(db.select).toHaveBeenCalledTimes(2);
     });
 
+    it('returns null when a historical hash matches different books', async () => {
+      db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(
+        makeQueryChain([
+          { id: 10, bookId: 20, libraryId: 1, format: 'epub' },
+          { id: 11, bookId: 21, libraryId: 1, format: 'epub' },
+        ]),
+      );
+
+      await expect(repo.resolveBookFileByHash('oldhash', null)).resolves.toBeNull();
+      expect(db.select).toHaveBeenCalledTimes(2);
+    });
+
     it('falls back to a user-scoped manual link after direct and history lookups miss', async () => {
       const file = { id: 10, bookId: 20, libraryId: 1, format: 'cbz' };
       db.select
@@ -200,6 +250,64 @@ describe('KoreaderRepository', () => {
       expect(db.select).toHaveBeenCalledTimes(1);
     });
 
+    it('resolves duplicate current hash rows when they belong to one book', async () => {
+      db.select.mockReturnValueOnce(
+        makeQueryChain([
+          { hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' },
+          { hash: 'current', bookFileId: 12, bookId: 21, libraryId: 31, format: 'epub' },
+        ]),
+      );
+
+      const result = await repo.resolveBookFilesByHashes(['current'], null);
+
+      expect(result.get('current')).toEqual({ bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' });
+      expect(db.select).toHaveBeenCalledTimes(1);
+    });
+
+    it('omits a current hash that matches different books', async () => {
+      db.select.mockReturnValueOnce(
+        makeQueryChain([
+          { hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' },
+          { hash: 'current', bookFileId: 12, bookId: 22, libraryId: 31, format: 'epub' },
+        ]),
+      );
+
+      const result = await repo.resolveBookFilesByHashes(['current'], null);
+
+      expect(result.has('current')).toBe(false);
+      expect(db.select).toHaveBeenCalledTimes(1);
+    });
+
+    it('uses a user-scoped manual link for an ambiguous current hash', async () => {
+      db.select
+        .mockReturnValueOnce(
+          makeQueryChain([
+            { hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' },
+            { hash: 'current', bookFileId: 12, bookId: 22, libraryId: 31, format: 'epub' },
+          ]),
+        )
+        .mockReturnValueOnce(makeQueryChain([{ hash: 'current', bookFileId: 12, bookId: 22, libraryId: 31, format: 'epub' }]));
+
+      const result = await repo.resolveBookFilesByHashes(['current'], null, 7);
+
+      expect(result.get('current')).toEqual({ bookFileId: 12, bookId: 22, libraryId: 31, format: 'epub' });
+      expect(db.select).toHaveBeenCalledTimes(2);
+    });
+
+    it('omits a historical hash that matches different books', async () => {
+      db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(
+        makeQueryChain([
+          { hash: 'old', bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' },
+          { hash: 'old', bookFileId: 12, bookId: 22, libraryId: 31, format: 'epub' },
+        ]),
+      );
+
+      const result = await repo.resolveBookFilesByHashes(['old'], null);
+
+      expect(result.has('old')).toBe(false);
+      expect(db.select).toHaveBeenCalledTimes(2);
+    });
+
     it('resolves remaining hashes from user-scoped manual links', async () => {
       db.select
         .mockReturnValueOnce(makeQueryChain([{ hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' }]))

server/src/modules/koreader/koreader.repository.ts

@@ -24,6 +24,39 @@ type KoreaderHashLinkMetadata = {
   lastOpen?: number | null;
 };
 
+type HashedResolvedBookFile = ResolvedBookFileByHashes & { hash: string | null };
+
+function resolveUniqueBook<T extends { bookId: number }>(rows: T[]): T | null {
+  const first = rows[0];
+  if (!first) return null;
+  return rows.every((row) => row.bookId === first.bookId) ? first : null;
+}
+
+function addUnambiguousHashMatches(rows: HashedResolvedBookFile[], result: Map<string, ResolvedBookFileByHashes>): Set<string> {
+  const rowsByHash = new Map<string, HashedResolvedBookFile[]>();
+
+  for (const row of rows) {
+    if (!row.hash) continue;
+    const matches = rowsByHash.get(row.hash);
+    if (matches) matches.push(row);
+    else rowsByHash.set(row.hash, [row]);
+  }
+
+  for (const [hash, matches] of rowsByHash) {
+    const resolved = resolveUniqueBook(matches);
+    if (resolved) {
+      result.set(hash, {
+        bookFileId: resolved.bookFileId,
+        bookId: resolved.bookId,
+        libraryId: resolved.libraryId,
+        format: resolved.format,
+      });
+    }
+  }
+
+  return new Set(rowsByHash.keys());
+}
+
 export interface ReadingProgressUpsert {
   bookFileId: number;
   userId: number;
@@ -78,49 +111,51 @@ export class KoreaderRepository {
     await this.db.delete(schema.koreaderUsers).where(eq(schema.koreaderUsers.userId, userId));
   }
 
-  // A file hash is not unique: the same content in two places is two book file rows, and one
-  // historical hash can belong to several of them. The oldest row wins so a hash always resolves
-  // to the same target, both across requests and between this and resolveBookFilesByHashes. An
-  // unordered pick would move a device's sync target under it and strand data on the loser.
+  // KOReader's partial hash can collide for different files. Multiple matching files are safe only
+  // when they belong to the same book; otherwise a user-scoped manual link must disambiguate them.
   async resolveBookFileByHash(hash: string, accessibleLibraryIds: number[] | null, userId?: number): Promise<ResolvedBookFileByHash | null> {
     if (accessibleLibraryIds !== null && accessibleLibraryIds.length === 0) return null;
 
     const libraryFilter = accessibleLibraryIds ? inArray(schema.books.libraryId, accessibleLibraryIds) : undefined;
 
-    const [byFileHash] = await this.db
-      .select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId, format: schema.bookFiles.format })
+    const byFileHash = await this.db
+      .selectDistinctOn([schema.bookFiles.bookId], {
+        id: schema.bookFiles.id,
+        bookId: schema.bookFiles.bookId,
+        libraryId: schema.books.libraryId,
+        format: schema.bookFiles.format,
+      })
       .from(schema.bookFiles)
       .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
       .where(and(eq(schema.bookFiles.fileHash, hash), libraryFilter))
-      .orderBy(asc(schema.bookFiles.id))
-      .limit(1);
+      .orderBy(asc(schema.bookFiles.bookId), asc(schema.bookFiles.id))
+      .limit(2);
 
-    if (byFileHash) return byFileHash;
+    if (byFileHash.length > 0) {
+      return resolveUniqueBook(byFileHash) ?? (userId === undefined ? null : this.resolveManualBookFileByHash(hash, accessibleLibraryIds, userId));
+    }
 
-    const [byFileHashHistory] = await this.db
-      .select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId, format: schema.bookFiles.format })
+    const byFileHashHistory = await this.db
+      .selectDistinctOn([schema.bookFiles.bookId], {
+        id: schema.bookFiles.id,
+        bookId: schema.bookFiles.bookId,
+        libraryId: schema.books.libraryId,
+        format: schema.bookFiles.format,
+      })
       .from(schema.bookFileHashHistory)
       .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.bookFileHashHistory.bookFileId))
       .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
       .where(and(eq(schema.bookFileHashHistory.fileHash, hash), libraryFilter))
-      .orderBy(asc(schema.bookFiles.id))
-      .limit(1);
+      .orderBy(asc(schema.bookFiles.bookId), asc(schema.bookFiles.id))
+      .limit(2);
 
-    if (byFileHashHistory) return byFileHashHistory;
-
-    if (userId !== undefined) {
-      const [byManualLink] = await this.db
-        .select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId, format: schema.bookFiles.format })
-        .from(schema.koreaderBookHashLinks)
-        .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.koreaderBookHashLinks.bookFileId))
-        .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
-        .where(and(eq(schema.koreaderBookHashLinks.userId, userId), eq(schema.koreaderBookHashLinks.hash, hash), libraryFilter))
-        .limit(1);
-
-      if (byManualLink) return byManualLink;
+    if (byFileHashHistory.length > 0) {
+      return (
+        resolveUniqueBook(byFileHashHistory) ?? (userId === undefined ? null : this.resolveManualBookFileByHash(hash, accessibleLibraryIds, userId))
+      );
     }
 
-    return null;
+    return userId === undefined ? null : this.resolveManualBookFileByHash(hash, accessibleLibraryIds, userId);
   }
 
   async resolveBookFilesByHashes(
@@ -147,36 +182,28 @@ export class KoreaderRepository {
       .where(and(inArray(schema.bookFiles.fileHash, hashes), libraryFilter))
       .orderBy(asc(schema.bookFiles.id));
 
-    for (const row of direct) {
-      if (row.hash && !result.has(row.hash)) {
-        result.set(row.hash, { bookFileId: row.bookFileId, bookId: row.bookId, libraryId: row.libraryId, format: row.format });
-      }
+    const directHashes = addUnambiguousHashMatches(direct, result);
+
+    const missing = hashes.filter((hash) => !directHashes.has(hash));
+    if (missing.length > 0) {
+      const history = await this.db
+        .select({
+          hash: schema.bookFileHashHistory.fileHash,
+          bookFileId: schema.bookFiles.id,
+          bookId: schema.bookFiles.bookId,
+          libraryId: schema.books.libraryId,
+          format: schema.bookFiles.format,
+        })
+        .from(schema.bookFileHashHistory)
+        .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.bookFileHashHistory.bookFileId))
+        .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
+        .where(and(inArray(schema.bookFileHashHistory.fileHash, missing), libraryFilter))
+        .orderBy(asc(schema.bookFiles.id));
+
+      addUnambiguousHashMatches(history, result);
     }
 
-    const missing = hashes.filter((hash) => !result.has(hash));
-    if (missing.length === 0) return result;
-
-    const history = await this.db
-      .select({
-        hash: schema.bookFileHashHistory.fileHash,
-        bookFileId: schema.bookFiles.id,
-        bookId: schema.bookFiles.bookId,
-        libraryId: schema.books.libraryId,
-        format: schema.bookFiles.format,
-      })
-      .from(schema.bookFileHashHistory)
-      .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.bookFileHashHistory.bookFileId))
-      .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
-      .where(and(inArray(schema.bookFileHashHistory.fileHash, missing), libraryFilter))
-      .orderBy(asc(schema.bookFiles.id));
-
-    for (const row of history) {
-      if (!result.has(row.hash)) {
-        result.set(row.hash, { bookFileId: row.bookFileId, bookId: row.bookId, libraryId: row.libraryId, format: row.format });
-      }
-    }
-
-    const stillMissing = missing.filter((hash) => !result.has(hash));
+    const stillMissing = hashes.filter((hash) => !result.has(hash));
     if (stillMissing.length === 0 || userId === undefined) return result;
 
     const manualLinks = await this.db
@@ -201,6 +228,23 @@ export class KoreaderRepository {
     return result;
   }
 
+  private async resolveManualBookFileByHash(
+    hash: string,
+    accessibleLibraryIds: number[] | null,
+    userId: number,
+  ): Promise<ResolvedBookFileByHash | null> {
+    const libraryFilter = accessibleLibraryIds ? inArray(schema.books.libraryId, accessibleLibraryIds) : undefined;
+    const [byManualLink] = await this.db
+      .select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId, format: schema.bookFiles.format })
+      .from(schema.koreaderBookHashLinks)
+      .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.koreaderBookHashLinks.bookFileId))
+      .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
+      .where(and(eq(schema.koreaderBookHashLinks.userId, userId), eq(schema.koreaderBookHashLinks.hash, hash), libraryFilter))
+      .limit(1);
+
+    return byManualLink ?? null;
+  }
+
   async upsertUnmatchedBooks(userId: number, candidates: KoreaderUnmatchedCandidate[], deviceId?: string): Promise<void> {
     if (candidates.length === 0) return;
 

server/test/koreader-progress-position.e2e-spec.ts

@@ -193,4 +193,26 @@ describe('KOReader progress position routing (e2e)', { timeout: 180_000 }, () =>
     expect(stored?.pageNumber).toBeNull();
     expect(stored?.percentage).toBeCloseTo(80, 5);
   });
+
+  it('does not route progress when the document hash matches different books', async () => {
+    const comicProgressBefore = await storedProgress(comic.bookFileId);
+    const epubProgressBefore = await storedProgress(epub.bookFileId);
+
+    await ctx.db.update(schema.bookFiles).set({ fileHash: comicHash }).where(eq(schema.bookFiles.id, epub.bookFileId));
+
+    try {
+      const response = await ctx.app.inject({
+        method: 'PUT',
+        url: '/api/v1/koreader/syncs/progress',
+        headers: deviceHeaders(),
+        payload: { document: comicHash, percentage: 0.99, progress: '999' },
+      });
+
+      expect(response.statusCode).toBe(404);
+      await expect(storedProgress(comic.bookFileId)).resolves.toEqual(comicProgressBefore);
+      await expect(storedProgress(epub.bookFileId)).resolves.toEqual(epubProgressBefore);
+    } finally {
+      await ctx.db.update(schema.bookFiles).set({ fileHash: epubHash }).where(eq(schema.bookFiles.id, epub.bookFileId));
+    }
+  });
 });