From e40bc474bc65f1a167fc4012b28ce0ce65ea9575 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sun, 6 Sep 2026 18:54:58 +0200 Subject: [PATCH] fix(usage): let an operator clear a participation the collector never accepted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Probing the live collector to settle the delete-sequence question turned up something else: usage.picpeak.app answers a valid usage.v2 registration with INVALID_PACKET while the identical v1 flow is accepted. It does not speak v2 yet — which the deployment notes already require, but the consequence of getting that order wrong was worse than "reports do not send". Opting in to v2 against a v1-only collector left the installation stuck. Registration was refused, so nothing existed at the collector at all; the row sat in activation_pending, disable moved it to deletion_pending, retry was futile forever, and enable refused because the row was not `disabled`. The abandon hatch added earlier did not apply: it was gated on SIGNING_KEY_UNREADABLE. So the most harmless possible failure — nothing registered anywhere — was the one an operator could not clear. The gate is now the property that actually matters: a participation the collector has provably never accepted (sequence 0, no receipt) with a failing delivery can be discarded, from activation_pending as well as deletion_pending. Its receipt records `never-registered` rather than an unconfirmed deletion, because nothing remote exists to be unsure about. A participation the collector *did* accept keeps the old narrow gate and its explicit warning — clearing local state while the collector still holds reports must stay a deliberate, warned-about act. A collector that rejects a registration or a deletion outright now reports SCHEMA_NOT_ACCEPTED instead of DELIVERY_FAILED, and the settings page says the collector does not accept this report version yet. Retrying cannot fix that, and sending the operator to look for a network fault they do not have was wrong. Verified end to end against the live collector: v2 opt-in reports SCHEMA_NOT_ACCEPTED, the exit is offered immediately, the receipt says never-registered, and joining again on v1 registers, reports and withdraws with a collector-confirmed deletion. --- .../services/usageServiceKeyRotation.test.js | 97 +++++++++++++++++++ backend/src/usage/UsageService.js | 68 +++++++++---- docs/PRODUCT_USAGE.md | 15 ++- .../settings/tabs/ProductUsageTab.tsx | 18 +++- frontend/src/i18n/locales/de.json | 3 + frontend/src/i18n/locales/en.json | 3 + frontend/src/services/productUsage.service.ts | 4 +- 7 files changed, 186 insertions(+), 22 deletions(-) diff --git a/backend/__tests__/services/usageServiceKeyRotation.test.js b/backend/__tests__/services/usageServiceKeyRotation.test.js index 53116fc9..18485d2f 100644 --- a/backend/__tests__/services/usageServiceKeyRotation.test.js +++ b/backend/__tests__/services/usageServiceKeyRotation.test.js @@ -260,3 +260,100 @@ describe('delivery backoff', () => { expect(Number(cleared.next_attempt_at)).toBe(0); }); }); + +/** + * The same dead end, reached the ordinary way. If an installation opts in to + * usage.v2 while the collector still only speaks usage.v1 — the deployment + * order the docs warn about — the registration is rejected outright. Nothing + * exists at the collector, and yet the operator could not clear the tab: + * disable moved to deletion_pending, retry was futile, enable refused, and the + * abandon hatch was gated on SIGNING_KEY_UNREADABLE, which this is not. + * + * Verified against the live collector before this was written: a valid v2 + * register is answered with INVALID_PACKET while the identical v1 flow is + * accepted. + */ +describe('a participation the collector never accepted', () => { + let db; + afterEach(async () => { if (db) await db.destroy(); db = null; }); + + const rejectingCollector = async (status) => { + db = await bootDb(); + await db.schema.createTable('product_usage_markers', (t) => { + t.string('feature', 60).primary(); + }); + const identity = generateIdentity(); + const service = new UsageService(db, { + secret: SECRET_A, + endpoint: 'https://usage.example.test', + bindingPath: `${require('os').tmpdir()}/usage-unreg-${Date.now()}-${Math.random()}.key`, + fetch: async () => ({ + ok: false, + status: 400, + headers: { get: () => null }, + body: (async function* () { yield Buffer.from(JSON.stringify({ error: 'INVALID_PACKET' })); })(), + }), + }); + await db('product_usage_state').where({ id: 1 }).update({ + status, + consent_version: 'usage-consent.v2', + installation_id: identity.installation_id, + public_key: identity.public_key, + private_key_encrypted: service.encrypt(identity.private_key), + sequence: 0, + pending_packet: JSON.stringify( + makePacket(identity, status === 'deletion_pending' ? 'delete' : 'register', 0, + status === 'deletion_pending' ? {} : { consent_version: 'usage-consent.v2' }, 'usage.v2') + ), + }); + return service; + }; + + it('names the rejection instead of blaming the network', async () => { + const service = await rejectingCollector('activation_pending'); + await service.tick({ force: true }); + expect((await service.state()).last_error).toBe('SCHEMA_NOT_ACCEPTED'); + }); + + it('offers the exit straight from activation_pending', async () => { + const service = await rejectingCollector('activation_pending'); + await service.tick({ force: true }); + const status = await service.status(); + expect(status.can_abandon).toBe(true); + expect(status.abandon_never_registered).toBe(true); + + await service.abandon(); + const after = await service.state(); + expect(after.status).toBe('disabled'); + expect(after.installation_id).toBeNull(); + // Provably nothing remote, so the receipt must not hedge. + expect(JSON.parse(after.privacy_receipts).last_abandonment.status) + .toBe('never-registered'); + }); + + it('offers it from deletion_pending too, once the withdrawal is also undeliverable', async () => { + const service = await rejectingCollector('deletion_pending'); + await service.tick({ force: true }); + expect((await service.status()).can_abandon).toBe(true); + await service.abandon(); + expect((await service.state()).status).toBe('disabled'); + }); + + it('never offers it while a registered participation could still be deleted remotely', async () => { + const service = await rejectingCollector('deletion_pending'); + // Something WAS accepted once: the collector may still hold reports, so + // clearing local state silently would be a lie. + await db('product_usage_state').where({ id: 1 }).update({ + sequence: 3, + last_receipt: JSON.stringify({ status: 'accepted' }), + last_error: 'DELIVERY_FAILED', + }); + expect((await service.status()).can_abandon).toBe(false); + await expect(service.abandon()).rejects.toThrow(/cannot be completed/); + }); + + it('does not offer it before a delivery has actually failed', async () => { + const service = await rejectingCollector('activation_pending'); + expect((await service.status()).can_abandon).toBe(false); + }); +}); diff --git a/backend/src/usage/UsageService.js b/backend/src/usage/UsageService.js index ed4ba145..20037391 100644 --- a/backend/src/usage/UsageService.js +++ b/backend/src/usage/UsageService.js @@ -106,6 +106,29 @@ const parse = (value) => { }; class UsageService { + // The collector has provably never accepted anything from this identity: + // no packet was ever acknowledged, so there is nothing remote to delete. + // Abandoning such a participation is harmless, which is why it may be + // offered without the warning the registered case needs. + static neverAccepted(state) { + return Number(state?.sequence || 0) === 0 && !state?.last_receipt; + } + + // The two ways a participation can reach a state no amount of retrying will + // resolve. Kept as one predicate so the settings page and the endpoint can + // never disagree about whether the exit is available. + static abandonable(state) { + if (!['activation_pending', 'deletion_pending'].includes(state?.status)) + return false; + // The signing key is gone: the delete packet can never be produced. + if (state.last_error === 'SIGNING_KEY_UNREADABLE') return true; + // Or the collector never accepted anything and a delivery is failing — + // the ordinary shape of "opted in against a collector that does not speak + // this report version yet". Nothing is registered remotely, so clearing + // the local state costs nothing and is the only way out of the tab. + return Boolean(state.last_error) && UsageService.neverAccepted(state); + } + schemaVersion(state) { return state?.consent_version === CURRENT_CONSENT_VERSION ? CURRENT_SCHEMA_VERSION : 'usage.v1'; @@ -254,14 +277,12 @@ class UsageService { Number(state.next_attempt_at || 0) > this.now() ? Number(state.next_attempt_at) : null, - // The one failure the operator cannot retry their way out of: the - // signing key is unreadable, so the delete packet can never be signed. - // Without this flag the settings page has no way to offer the only - // remaining exit (abandon), and the install sits in deletion_pending - // forever. - can_abandon: - state.status === 'deletion_pending' && - state.last_error === 'SIGNING_KEY_UNREADABLE', + // Whether the only remaining exit should be offered. Without it the tab + // shows a permanent error and no control that can clear it. + can_abandon: UsageService.abandonable(state), + // Distinguishes the harmless case (nothing was ever registered, so + // abandoning deletes nothing remote) from the one that needs a warning. + abandon_never_registered: UsageService.neverAccepted(state), pending_action: state.pending_packet ? JSON.parse(state.pending_packet).action : null, @@ -433,13 +454,11 @@ class UsageService { // unconfirmed deletion is its own decision, not a side effect of opting in. async abandon() { await this.locked(async (state) => { - if ( - state.status !== 'deletion_pending' || - state.last_error !== 'SIGNING_KEY_UNREADABLE' - ) + if (!UsageService.abandonable(state)) throw new ConflictError( - 'Only an unsignable withdrawal can be abandoned' + 'Only a participation that cannot be completed can be abandoned' ); + const neverRegistered = UsageService.neverAccepted(state); await fs.unlink(this.bindingPath).catch((error) => { if (error.code !== 'ENOENT') throw error; }); @@ -448,7 +467,8 @@ class UsageService { ? JSON.parse(state.privacy_receipts) : {}; await this.db('product_usage_state') - .where({ id: 1, status: 'deletion_pending' }) + .where({ id: 1 }) + .whereIn('status', ['activation_pending', 'deletion_pending']) .update({ status: 'disabled', installation_id: null, @@ -471,8 +491,15 @@ class UsageService { kind: 'abandonment', receipt_id: crypto.randomUUID(), confirmed_at: new Date(this.now()).toISOString(), - status: 'collector-unconfirmed', - reason: 'SIGNING_KEY_UNREADABLE', + // Two different truths, and the receipt has to tell them apart. + // Nothing was ever accepted -> there is provably nothing at the + // collector. Otherwise the collector may still hold reports and + // was never told to delete them; saying "unconfirmed" is the + // only honest wording for that. + status: neverRegistered + ? 'never-registered' + : 'collector-unconfirmed', + reason: state.last_error, installation_id: state.installation_id, scope: ['local identity', 'local markers', 'local key material'] } @@ -681,11 +708,18 @@ class UsageService { 'NOT_REGISTERED', 'PACKET_CONFLICT' ].includes(error.code); + // A collector that answers INVALID_PACKET to a registration or a deletion + // does not understand the wire version we speak — most often because it + // has not been upgraded to usage.v2 yet. Retrying cannot fix that, and + // reporting it as DELIVERY_FAILED sent the operator looking for a + // network problem they do not have. const code = conflict ? error.code : error.code === 'SIGNING_KEY_UNREADABLE' ? 'SIGNING_KEY_UNREADABLE' - : 'DELIVERY_FAILED'; + : rejected && ['register', 'delete'].includes(packet.action) + ? 'SCHEMA_NOT_ACCEPTED' + : 'DELIVERY_FAILED'; await this.db('product_usage_state') .where({ id: 1 }) .update({ last_error: code }); diff --git a/docs/PRODUCT_USAGE.md b/docs/PRODUCT_USAGE.md index d60d65f7..df9bb4fc 100644 --- a/docs/PRODUCT_USAGE.md +++ b/docs/PRODUCT_USAGE.md @@ -77,7 +77,20 @@ state. It erases the local identity, key material and markers and records an abandonment receipt marked `collector-unconfirmed`: the collector was never told, so it keeps the reports already accepted, and the receipt says so rather than claiming a deletion that did not happen. Participation can be started -again afterwards with a fresh identity. Keys live in a dedicated database +again afterwards with a fresh identity. + +The same exit covers the other way a participation can become impossible to +finish: a collector that rejects the packet outright. Opting in to usage.v2 +against a collector that still only speaks usage.v1 — the deployment order +this document warns about above — is answered with `INVALID_PACKET`, which is +surfaced as `SCHEMA_NOT_ACCEPTED` rather than a generic delivery failure, +because retrying cannot resolve it. Nothing is registered in that case, so +**Discard local identity** is offered immediately and its receipt records +`never-registered` rather than an unconfirmed deletion. The exit is never +offered while a participation the collector *did* accept could still be +deleted remotely; that case keeps the explicit warning. + +Keys live in a dedicated database table, not the generic readable settings. A random mode-0600 file at `getStoragePath()/usage-instance.key` binds the database to its local storage. diff --git a/frontend/src/features/settings/tabs/ProductUsageTab.tsx b/frontend/src/features/settings/tabs/ProductUsageTab.tsx index b8ef2b18..fc731ed6 100644 --- a/frontend/src/features/settings/tabs/ProductUsageTab.tsx +++ b/frontend/src/features/settings/tabs/ProductUsageTab.tsx @@ -278,7 +278,9 @@ export default function ProductUsageTab() { {t( data.last_error === 'SIGNING_KEY_UNREADABLE' ? 'productUsage.signingKeyUnreadable' - : 'productUsage.deliveryProblem' + : data.last_error === 'SCHEMA_NOT_ACCEPTED' + ? 'productUsage.schemaNotAccepted' + : 'productUsage.deliveryProblem' )}

)} @@ -296,7 +298,13 @@ export default function ProductUsageTab() { // The one dead end the operator cannot retry out of. Offered only // here, and worded so nobody mistakes it for a confirmed deletion.
-

{t('productUsage.abandonExplanation')}

+

+ {t( + data.abandon_never_registered + ? 'productUsage.abandonExplanationUnregistered' + : 'productUsage.abandonExplanation' + )} +