fix(kobo): scope plugin update gate to requesting device

Evaluate identified KOReader clients independently so a stale sibling device does not suppress safe updates. Keep the conservative account-wide gate for legacy or unusable client identities.

Fixes #1519

koreader-plugin/bookorbit.koplugin/bookorbit_api.lua

@@ -193,7 +193,7 @@ end
 
 -- Returns decoded_body on success, or nil, err_code, decoded_error_body.
 -- err_code is a number for HTTP errors and a string for transport errors.
-function BookOrbitApi:requestBlocking(method, path, body)
+function BookOrbitApi:requestBlocking(method, path, body, extra_headers)
     local sink = {}
     local request = {
         url = self.server_url .. path,
@@ -206,6 +206,10 @@ function BookOrbitApi:requestBlocking(method, path, body)
         },
     }
 
+    for name, value in pairs(extra_headers or {}) do
+        request.headers[name] = value
+    end
+
     if body then
         local body_json, encode_err = rapidjson.encode(body, ENCODE_OPTIONS)
         if not body_json then
@@ -287,13 +291,13 @@ function BookOrbitApi:runInSubprocess(fn, trap_widget)
     return true, result
 end
 
-function BookOrbitApi:request(method, path, body)
+function BookOrbitApi:request(method, path, body, extra_headers)
     if not self:canForkSubprocess() then
-        return self:requestBlocking(method, path, body)
+        return self:requestBlocking(method, path, body, extra_headers)
     end
 
     local completed, result = self:runInSubprocess(function()
-        return self:requestBlocking(method, path, body)
+        return self:requestBlocking(method, path, body, extra_headers)
     end)
     if not completed then
         return nil, "background_request_interrupted"
@@ -664,7 +668,18 @@ end
 -- Plugin self-update endpoints
 
 function BookOrbitApi:getPluginVersion()
-    return self:request("GET", "/koreader/plugin/version")
+    local headers = {}
+    local valid_device_id = type(self.device_id) == "string"
+        and #self.device_id <= 100
+        and self.device_id:match("^[A-Za-z0-9-]+$") ~= nil
+    local valid_plugin_version = type(self.plugin_version) == "string"
+        and #self.plugin_version <= 20
+        and self.plugin_version:find("[\r\n]") == nil
+    if valid_device_id and valid_plugin_version then
+        headers["x-bookorbit-device-id"] = self.device_id
+        headers["x-bookorbit-plugin-version"] = self.plugin_version
+    end
+    return self:request("GET", "/koreader/plugin/version", nil, headers)
 end
 
 function BookOrbitApi:downloadPluginUpdate(local_path, opts)

koreader-plugin/spec/bookorbit_api_test.lua

@@ -70,12 +70,14 @@ package.loaded["ltn12"] = {
 }
 
 local last_request_url
+local last_request_headers
 local request_count = 0
 package.loaded["socket.http"] = {
     request = function(request)
         request_count = request_count + 1
         request_ran_in_subprocess = in_subprocess
         last_request_url = request.url
+        last_request_headers = request.headers
         if mock_http_body then
             request.sink(mock_http_body)
         end
@@ -179,6 +181,37 @@ assertEqual(last_request_url,
     "https://bookorbit.example.com/api/v1/koreader/plugin/catalog/dashboard/sections/up-next-in-series",
     "the dashboard-section endpoint is addressed by source type")
 
+client:getPluginVersion()
+assertEqual(last_request_headers["x-bookorbit-device-id"], nil,
+    "a legacy client sends no partial self-update identity")
+assertEqual(last_request_headers["x-bookorbit-plugin-version"], nil,
+    "a legacy client sends no partial self-update version")
+
+local partial_identity_client = BookOrbitApi.new{
+    server_url = "https://bookorbit.example.com/api/v1",
+    username = "reader",
+    userkey = "secret",
+    device_id = "device-id",
+}
+partial_identity_client:getPluginVersion()
+assertEqual(last_request_headers["x-bookorbit-device-id"], nil,
+    "an incomplete identity sends no device header")
+assertEqual(last_request_headers["x-bookorbit-plugin-version"], nil,
+    "an incomplete identity sends no version header")
+
+local unsafe_identity_client = BookOrbitApi.new{
+    server_url = "https://bookorbit.example.com/api/v1",
+    username = "reader",
+    userkey = "secret",
+    device_id = "device-id\r\ninjected",
+    plugin_version = "1.5.4",
+}
+unsafe_identity_client:getPluginVersion()
+assertEqual(last_request_headers["x-bookorbit-device-id"], nil,
+    "an unsafe device id sends no identity headers")
+assertEqual(last_request_headers["x-bookorbit-plugin-version"], nil,
+    "an unsafe device id sends no version header")
+
 local match_client = BookOrbitApi.new{
     server_url = "https://bookorbit.example.com/api/v1",
     username = "reader",
@@ -193,6 +226,12 @@ local invalid_hash = "not-an-md5"
 local oversized_title = string.rep("t", 501)
 local oversized_authors = string.rep("a", 1001)
 
+match_client:getPluginVersion()
+assertEqual(last_request_headers["x-bookorbit-device-id"], "device-id",
+    "the update check identifies the requesting device")
+assertEqual(last_request_headers["x-bookorbit-plugin-version"], "1.5.1",
+    "the update check reports the requesting plugin version")
+
 mock_http_body = "{\"ok\":true}"
 mock_http_code = 200
 body, err = match_client:matchCheck({ hash_a, invalid_hash, hash_b }, {
@@ -273,6 +312,22 @@ assertEqual(err, nil, "background request preserves success result")
 assertEqual(subprocess_calls, 1, "wrapped background request uses subprocess")
 assertEqual(request_ran_in_subprocess, true, "HTTP request runs inside subprocess task")
 
+local background_identity_client = BookOrbitApi.new{
+    server_url = "https://bookorbit.example.com/api/v1",
+    username = "reader",
+    userkey = "secret",
+    device_id = "background-device",
+    plugin_version = "1.5.4",
+    background_requests = true,
+}
+body, err = background_identity_client:getPluginVersion()
+assertEqual(err, nil, "background update check succeeds")
+assertEqual(subprocess_calls, 2, "background update check uses subprocess")
+assertEqual(last_request_headers["x-bookorbit-device-id"], "background-device",
+    "the subprocess preserves the requesting device header")
+assertEqual(last_request_headers["x-bookorbit-plugin-version"], "1.5.4",
+    "the subprocess preserves the requesting version header")
+
 subprocess_result_mode = "missing"
 body, err = background_client:auth()
 assertEqual(body, nil, "missing subprocess payload has no response body")
@@ -288,7 +343,7 @@ wrapped = false
 request_ran_in_subprocess = false
 body, err = background_client:auth()
 assertEqual(body.ok, true, "unwrapped request falls back safely")
-assertEqual(subprocess_calls, 3, "unwrapped request does not start subprocess")
+assertEqual(subprocess_calls, 4, "unwrapped request does not start subprocess")
 assertEqual(request_ran_in_subprocess, false, "unwrapped fallback runs in current process")
 
 print("bookorbit_api_test.lua: ok")

server/src/modules/koreader/koreader-package.service.test.ts

@@ -6,6 +6,7 @@ import * as unzipper from 'unzipper';
 import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { KoreaderPackageService } from './koreader-package.service';
+import { SELF_UPDATE_MIN_PLUGIN_VERSION } from './koreader-plugin-update.util';
 
 function makeCredentialsRow(overrides?: Record<string, unknown>) {
   return {
@@ -205,6 +206,63 @@ describe('KoreaderPackageService', () => {
   });
 
   describe('getVersionInfoForSelfUpdate', () => {
+    it('evaluates a current identified device independently of stale devices on the account', async () => {
+      mockPluginRepo.listDevicePluginVersions.mockResolvedValue(['1.3.1']);
+
+      const result = await service.getVersionInfoForSelfUpdate(7, {
+        deviceId: 'current-device',
+        pluginVersion: '1.5.4',
+      });
+
+      expect(result.pluginVersion).toBe('1.2.3');
+      expect(mockPluginRepo.listDevicePluginVersions).not.toHaveBeenCalled();
+    });
+
+    it('allows the first self-update-safe version for an identified device', async () => {
+      const result = await service.getVersionInfoForSelfUpdate(7, {
+        deviceId: 'current-device',
+        pluginVersion: SELF_UPDATE_MIN_PLUGIN_VERSION,
+      });
+
+      expect(result.pluginVersion).toBe('1.2.3');
+      expect(mockPluginRepo.listDevicePluginVersions).not.toHaveBeenCalled();
+    });
+
+    it('withholds the version from an identified device whose updater can crash', async () => {
+      mockPluginRepo.listDevicePluginVersions.mockResolvedValue(['1.5.4']);
+
+      const result = await service.getVersionInfoForSelfUpdate(7, {
+        deviceId: 'outdated-device',
+        pluginVersion: '1.3.1',
+      });
+
+      expect(result.pluginVersion).toBe('unknown');
+      expect(mockPluginRepo.listDevicePluginVersions).not.toHaveBeenCalled();
+    });
+
+    it.each(['', 'nightly'])('withholds the version for an identified device reporting %j', async (pluginVersion) => {
+      const result = await service.getVersionInfoForSelfUpdate(7, {
+        deviceId: 'current-device',
+        pluginVersion,
+      });
+
+      expect(result.pluginVersion).toBe('unknown');
+      expect(mockPluginRepo.listDevicePluginVersions).not.toHaveBeenCalled();
+    });
+
+    it.each([
+      { client: { pluginVersion: '1.5.4' }, label: 'a missing device id' },
+      { client: { deviceId: 'current-device' }, label: 'a missing plugin version' },
+      { client: { deviceId: '../current-device', pluginVersion: '1.5.4' }, label: 'an invalid device id' },
+    ])('retains the user-wide fallback for $label', async ({ client }) => {
+      mockPluginRepo.listDevicePluginVersions.mockResolvedValue(['1.3.1']);
+
+      const result = await service.getVersionInfoForSelfUpdate(7, client);
+
+      expect(result.pluginVersion).toBe('unknown');
+      expect(mockPluginRepo.listDevicePluginVersions).toHaveBeenCalledWith(7);
+    });
+
     it('returns the real plugin version when every device can self-update', async () => {
       mockPluginRepo.listDevicePluginVersions.mockResolvedValue(['1.4.0', '1.5.2']);
 

server/src/modules/koreader/koreader-package.service.ts

@@ -8,6 +8,7 @@ import { join, resolve } from 'path';
 import type { KoreaderPluginCapability, KoreaderPluginVersionInfo } from '@bookorbit/types';
 import { appConfig } from '../../config/config';
 import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
+import { KOREADER_DEVICE_ID_REGEX } from './dto/koreader-device-param.dto';
 import { KoreaderPluginRepository } from './koreader-plugin.repository';
 import { SELF_UPDATE_MIN_PLUGIN_VERSION, pluginRequiresManualUpdate } from './koreader-plugin-update.util';
 import { KoreaderRepository } from './koreader.repository';
@@ -17,6 +18,11 @@ const SELF_UPDATE_GATE_EVENT = 'koreader.plugin_self_update_gate';
 const PLUGIN_FOLDER = 'bookorbit.koplugin';
 const PROVISION_FILE = 'bookorbit_provision.lua';
 
+interface SelfUpdateClientIdentity {
+  deviceId?: string;
+  pluginVersion?: string;
+}
+
 // Wire features this server advertises. The plugin selects a new route only
 // when its name appears here, so a downgraded server transparently returns the
 // plugin to its legacy path.
@@ -94,17 +100,18 @@ export class KoreaderPackageService {
    * Withholds `pluginVersion` from users who still run a plugin that would crash
    * KOReader trying to apply the update. Those clients treat "unknown" as "the
    * server could not tell me" and show a message instead of the update prompt,
-   * so the crashing path stays unreachable. The endpoint carries no device id,
-   * only the user, so one stale device parks self-update for all of theirs; that
-   * is the safe direction, and it clears once the old device is updated by hand.
+   * so the crashing path stays unreachable. Current clients identify themselves
+   * on the request and are evaluated independently. Older clients carry no
+   * identity, so they retain the conservative user-wide gate.
    */
-  async getVersionInfoForSelfUpdate(userId: number): Promise<KoreaderPluginVersionInfo> {
+  async getVersionInfoForSelfUpdate(userId: number, client: SelfUpdateClientIdentity = {}): Promise<KoreaderPluginVersionInfo> {
     const startedAt = Date.now();
     const info = await this.getVersionInfo();
     // Only `null` means "every device can self-update". A device that reported a
     // blank version is a blocker whose label is falsy, so testing truthiness here
     // would open the gate for exactly the unparseable case it exists to catch.
-    const blockedBy = await this.findSelfUpdateBlocker(userId);
+    const reportedBlocker = this.getReportedSelfUpdateBlocker(client);
+    const blockedBy = reportedBlocker === undefined ? await this.findSelfUpdateBlocker(userId) : reportedBlocker;
     if (blockedBy === null) return info;
 
     this.logger.log(
@@ -113,6 +120,14 @@ export class KoreaderPackageService {
     return { ...info, pluginVersion: 'unknown' };
   }
 
+  /** `undefined` means the client did not provide a complete usable identity. */
+  private getReportedSelfUpdateBlocker(client: SelfUpdateClientIdentity): string | null | undefined {
+    if (typeof client.deviceId !== 'string' || !KOREADER_DEVICE_ID_REGEX.test(client.deviceId)) return undefined;
+    if (typeof client.pluginVersion !== 'string') return undefined;
+    if (!pluginRequiresManualUpdate(client.pluginVersion)) return null;
+    return client.pluginVersion || 'unreported';
+  }
+
   /**
    * The reported plugin version of one device that cannot self-update, or null
    * when every device can. Versions that are absent or blank report as

server/src/modules/koreader/koreader-plugin.controller.test.ts

@@ -157,17 +157,31 @@ describe('KoreaderPluginController', () => {
     expect(pluginService.sweepComplete).toHaveBeenCalledWith(user, dto);
   });
 
-  it('getVersion scopes the version check to the calling user so self-update can be gated', async () => {
+  it('getVersion passes the authenticated user and device identity to the self-update gate', async () => {
     const { controller, packageService } = makeController();
     const user = { id: 7 } as never;
 
-    const result = await controller.getVersion(user);
+    const result = await controller.getVersion(user, 'device-1', '1.5.4');
 
-    expect(packageService.getVersionInfoForSelfUpdate).toHaveBeenCalledWith(7);
+    expect(packageService.getVersionInfoForSelfUpdate).toHaveBeenCalledWith(7, {
+      deviceId: 'device-1',
+      pluginVersion: '1.5.4',
+    });
     expect(packageService.getVersionInfo).not.toHaveBeenCalled();
     expect(result).toEqual({ pluginVersion: '0.4.0', serverVersion: '1.0.0' });
   });
 
+  it('getVersion preserves absent identity headers for the legacy fallback', async () => {
+    const { controller, packageService } = makeController();
+
+    await controller.getVersion({ id: 7 } as never, undefined, undefined);
+
+    expect(packageService.getVersionInfoForSelfUpdate).toHaveBeenCalledWith(7, {
+      deviceId: undefined,
+      pluginVersion: undefined,
+    });
+  });
+
   it('downloadUpdatePackage builds a raw package and streams it with correct headers', async () => {
     const { controller, packageService } = makeController();
     const user = { id: 7 } as never;

server/src/modules/koreader/koreader-plugin.controller.ts

@@ -1,4 +1,4 @@
-import { Body, Controller, Get, Post, Res, UseGuards } from '@nestjs/common';
+import { Body, Controller, Get, Headers, Post, Res, UseGuards } from '@nestjs/common';
 import type { FastifyReply } from 'fastify';
 
 import { CurrentUser } from '../../common/decorators/current-user.decorator';
@@ -89,8 +89,12 @@ export class KoreaderPluginController {
   }
 
   @Get('version')
-  getVersion(@CurrentUser() user: RequestUser) {
-    return this.packageService.getVersionInfoForSelfUpdate(user.id);
+  getVersion(
+    @CurrentUser() user: RequestUser,
+    @Headers('x-bookorbit-device-id') deviceId?: string,
+    @Headers('x-bookorbit-plugin-version') pluginVersion?: string,
+  ) {
+    return this.packageService.getVersionInfoForSelfUpdate(user.id, { deviceId, pluginVersion });
   }
 
   @Get('package')