diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_api.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_api.lua index c030b31a5..f2671ad09 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_api.lua +++ b/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) diff --git a/koreader-plugin/spec/bookorbit_api_test.lua b/koreader-plugin/spec/bookorbit_api_test.lua index 5ca41544c..fc96f4369 100644 --- a/koreader-plugin/spec/bookorbit_api_test.lua +++ b/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") diff --git a/server/src/modules/koreader/koreader-package.service.test.ts b/server/src/modules/koreader/koreader-package.service.test.ts index fc5a88f84..8e70209af 100644 --- a/server/src/modules/koreader/koreader-package.service.test.ts +++ b/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) { 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']); diff --git a/server/src/modules/koreader/koreader-package.service.ts b/server/src/modules/koreader/koreader-package.service.ts index a60269ed2..ac7b8eaab 100644 --- a/server/src/modules/koreader/koreader-package.service.ts +++ b/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 { + async getVersionInfoForSelfUpdate(userId: number, client: SelfUpdateClientIdentity = {}): Promise { 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 diff --git a/server/src/modules/koreader/koreader-plugin.controller.test.ts b/server/src/modules/koreader/koreader-plugin.controller.test.ts index 703770061..e0187769d 100644 --- a/server/src/modules/koreader/koreader-plugin.controller.test.ts +++ b/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; diff --git a/server/src/modules/koreader/koreader-plugin.controller.ts b/server/src/modules/koreader/koreader-plugin.controller.ts index 500103bc8..0ded5ae82 100644 --- a/server/src/modules/koreader/koreader-plugin.controller.ts +++ b/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')