From d17562ac6029c90a39a46eab72e9f5f1c1aa4c43 Mon Sep 17 00:00:00 2001 From: Anso Date: Sat, 2 May 2026 00:39:41 -0400 Subject: [PATCH] fix(license): reject activation when LS response is missing instance.id (#867) LicenseService.activate() previously stored data.instance?.id || '' on success. If LS ever returned activated:true without an instance.id (malformed response, API change, transient bug), the user saw "Activated successfully" but every subsequent validate() and deactivate() short-circuited on the empty license_instance_id with a generic "no active license" error. The activation appeared to succeed while leaving the install in a broken state. After the existing catalog-id guard, also require a non-empty data.instance.id and reject up front with a retry-friendly message if missing. The check costs nothing on the happy path (LS has historically always returned instance.id on success) and turns a silent state divergence into a clear, actionable error. Adds two tests covering instance object absent and instance.id empty string. Both assert mockSetSystemState was never called, which catches any future code that accidentally writes state above the guard. Adds a block comment above initialize() explaining the dual-name relationship that confused the original audit: instance_id is the local UUID we pass to LS as instance_name, license_instance_id is the LS-issued activation id we pass back as instance_id on validate and deactivate. Same area, swapped names, no overlap. --- .../license-service-id-validation.test.ts | 45 +++++++++++++++++++ backend/src/services/LicenseService.ts | 35 ++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/backend/src/__tests__/license-service-id-validation.test.ts b/backend/src/__tests__/license-service-id-validation.test.ts index 874d5f66..c5db3db3 100644 --- a/backend/src/__tests__/license-service-id-validation.test.ts +++ b/backend/src/__tests__/license-service-id-validation.test.ts @@ -152,6 +152,51 @@ describe('LicenseService.activate() - catalog ID guard', () => { expect(mockSetSystemState).not.toHaveBeenCalledWith('license_status', 'active'); }); + it('rejects activation when the LS response is missing the instance object', async () => { + // Storing an empty license_instance_id would silently break later + // validate() and deactivate() calls. Reject up front so the user + // sees a clear error instead of a deceptive "Activated successfully". + mockAxiosPost.mockResolvedValueOnce({ + data: { + activated: true, + license_key: { id: 1, status: 'active', key: 'k', activation_limit: 1, activation_usage: 1, created_at: '2026-01-01', expires_at: null }, + meta: { + store_id: SENCHO_LS_STORE_ID, + product_id: SENCHO_LS_PRODUCT_ID_SKIPPER, + variant_id: VARIANT_SKIPPER_MONTHLY, + variant_name: 'Skipper Monthly', + product_name: 'Sencho Skipper', + }, + // instance: omitted on purpose + }, + }); + const result = await svc.activate('NO-INSTANCE-OBJECT-KEY'); + expect(result.success).toBe(false); + expect(result.error).toBe('License server returned an incomplete activation. Please try again.'); + expect(mockSetSystemState).not.toHaveBeenCalled(); + }); + + it('rejects activation when LS returns instance with empty id', async () => { + mockAxiosPost.mockResolvedValueOnce({ + data: { + activated: true, + license_key: { id: 1, status: 'active', key: 'k', activation_limit: 1, activation_usage: 1, created_at: '2026-01-01', expires_at: null }, + instance: { id: '', name: 'test', created_at: '2026-01-01' }, + meta: { + store_id: SENCHO_LS_STORE_ID, + product_id: SENCHO_LS_PRODUCT_ID_ADMIRAL, + variant_id: VARIANT_ADMIRAL_LIFETIME, + variant_name: 'Admiral Lifetime', + product_name: 'Sencho Admiral', + }, + }, + }); + const result = await svc.activate('EMPTY-INSTANCE-ID-KEY'); + expect(result.success).toBe(false); + expect(result.error).toBe('License server returned an incomplete activation. Please try again.'); + expect(mockSetSystemState).not.toHaveBeenCalled(); + }); + it('writes nothing to system_state when the catalog guard rejects', async () => { // Stronger than checking individual keys: any future code that adds a // setSystemState() call above the guard would silently break the diff --git a/backend/src/services/LicenseService.ts b/backend/src/services/LicenseService.ts index 21c368bc..6f05350b 100644 --- a/backend/src/services/LicenseService.ts +++ b/backend/src/services/LicenseService.ts @@ -194,6 +194,24 @@ export class LicenseService { return LicenseService.instance; } + /** + * Two distinct identifiers live in `system_state` and the names are + * confusingly close, so for future maintainers (and audit agents): + * + * `instance_id` local UUID generated once on first boot. We + * send it to LS as the activation's + * `instance_name` (LS treats this as a + * free-form label, e.g. a hostname). + * + * `license_instance_id` the activation ID LS returns on /activate. + * We send it back to LS as the `instance_id` + * parameter on /validate and /deactivate. + * + * They are NOT redundant and NEITHER overwrites the other. Renaming + * `instance_id` to e.g. `local_install_id` would be clearer but would + * churn frontend consumers and migrations for no functional change. + */ + /** * Initialize the license service on startup. * Ensures an instance ID exists and starts periodic validation for active licenses. @@ -451,9 +469,24 @@ export class LicenseService { return { success: false, error: 'This license key is not valid for Sencho.' }; } + // Reject if LS did not return a usable instance id. Storing an + // empty license_instance_id would silently break later validate() + // and deactivate() calls, both of which short-circuit on a falsy + // instance id with a generic "no active license" error. Better to + // surface the broken activation immediately than ship a state where + // the user thinks they are activated but every subsequent call + // fails. LS has historically always returned data.instance.id on + // a successful activation; this is a defense against an API change + // or transient malformed response, not a routinely-hit branch. + const lsInstanceId = data.instance?.id; + if (!lsInstanceId) { + console.warn('[License] Activation rejected: LS response missing instance.id.'); + return { success: false, error: 'License server returned an incomplete activation. Please try again.' }; + } + // Store license data db.setSystemState('license_key', licenseKey); - db.setSystemState('license_instance_id', data.instance?.id || ''); + db.setSystemState('license_instance_id', lsInstanceId); this.setLicenseStatus('active'); db.setSystemState('license_last_validated', Date.now().toString());