fix(connect): clear persisted keys on sign-out and removal

packages/unraid-api-plugin-connect/src/__test__/mothership.events.test.ts

@@ -192,6 +192,19 @@ describe('MothershipHandler - Behavioral Tests', () => {
             expect(controllerStops).toHaveLength(1);
             expect(publishedMessages).toHaveLength(2);
         });
+
+        it('publishes cleared state when relay shutdown fails', async () => {
+            vi.mocked(mothershipController.stop).mockRejectedValueOnce(new Error('relay stop failed'));
+
+            await expect(handler.logout({})).rejects.toThrow();
+            expect(publishedMessages).toEqual([
+                { channel: GRAPHQL_PUBSUB_CHANNEL.SERVERS, data: { servers: [] } },
+                {
+                    channel: GRAPHQL_PUBSUB_CHANNEL.OWNER,
+                    data: { owner: { username: 'root', url: '', avatar: '' } },
+                },
+            ]);
+        });
     });
 
     describe('DDoS Prevention Behavior', () => {

packages/unraid-api-plugin-connect/src/config/config.persistence.ts

@@ -10,12 +10,29 @@ import { parse as parseIni } from 'ini';
 import type { MyServersConfig as LegacyConfig } from './my-servers.config.js';
 import { emptyMyServersConfig, MyServersConfig } from './connect.config.js';
 
+const pendingConnectWrites = new Map<string, Promise<void>>();
+
 @Injectable()
 export class ConnectConfigPersister extends ConfigFilePersister<MyServersConfig> {
     constructor(configService: ConfigService) {
         super(configService);
     }
 
+    public override async persist(
+        config = this.configService.get<MyServersConfig>(this.configKey())
+    ): Promise<boolean> {
+        const path = this.configPath();
+        const previous = pendingConnectWrites.get(path) ?? Promise.resolve();
+        const write = previous.then(() => super.persist(config));
+        const settled = write.then(() => undefined, () => undefined);
+        pendingConnectWrites.set(path, settled);
+        try {
+            return await write;
+        } finally {
+            if (pendingConnectWrites.get(path) === settled) pendingConnectWrites.delete(path);
+        }
+    }
+
     /**
      * @override
      * @returns The name of the config file.

packages/unraid-api-plugin-connect/src/config/connect.config.service.ts

@@ -1,10 +1,29 @@
 import { Injectable, Logger } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { OnEvent } from '@nestjs/event-emitter';
+import { readFile, unlink } from 'node:fs/promises';
+
+import { parse as parseIni } from 'ini';
+import { isMatch } from 'lodash-es';
 
 import { EVENTS } from '../helper/nest-tokens.js';
+import { ConnectConfigPersister } from './config.persistence.js';
 import { ConfigType, emptyMyServersConfig, MyServersConfig } from './connect.config.js';
 
+/** The identity was cleared, but removal of the legacy credential file failed. */
+export class ConnectLegacyCleanupError extends Error {
+    constructor(cause: unknown) {
+        super('Connect sign-out could not remove the legacy credential file', { cause });
+    }
+}
+
+/** The cleared identity was written, but its durable state could not be verified. */
+export class ConnectIdentityVerificationError extends Error {
+    constructor(cause: unknown) {
+        super('Connect sign-out could not verify the cleared identity', { cause });
+    }
+}
+
 @Injectable()
 export class ConnectConfigService {
     public readonly configKey = 'connect.config';
@@ -15,6 +34,60 @@ export class ConnectConfigService {
         return this.configService.getOrThrow<MyServersConfig>(this.configKey);
     }
 
+    async revokeCloudKey(): Promise<void> {
+        const apiKeys = new Set<string>();
+        const currentKey = this.getConfig().apikey;
+        if (currentKey) apiKeys.add(currentKey);
+
+        const legacyPath = this.configService.get<string>(
+            'PATHS_MY_SERVERS_CONFIG',
+            '/boot/config/plugins/dynamix.my.servers/myservers.cfg'
+        );
+        if (legacyPath) {
+            const legacyContent = await readFile(legacyPath, 'utf8').catch(
+                (error: NodeJS.ErrnoException) => {
+                    if (error.code === 'ENOENT') return null;
+                    throw error;
+                }
+            );
+            if (legacyContent !== null) {
+                const legacyConfig = parseIni(legacyContent);
+                const legacyKey: unknown = legacyConfig.remote?.apikey;
+                if (legacyKey != null && typeof legacyKey !== 'string') {
+                    throw new Error('Connect legacy key could not be read');
+                }
+                if (legacyKey) apiKeys.add(legacyKey);
+            }
+        }
+        if (apiKeys.size === 0) return;
+
+        const mothership = new URL(this.configService.getOrThrow<string>('MOTHERSHIP_GRAPHQL_LINK'));
+        if (mothership.protocol !== 'https:') {
+            throw new Error('Connect key revocation requires HTTPS');
+        }
+
+        const endpoint = new URL('/ks/server/self-unregister', mothership);
+        for (const apiKey of apiKeys) {
+            const response = await fetch(endpoint, {
+                method: 'POST',
+                headers: { 'x-api-key': apiKey },
+                redirect: 'error',
+                signal: AbortSignal.timeout(15_000),
+            });
+            if (!response.ok) throw new Error('Connect cloud key revocation failed');
+
+            const result: unknown = await response.json();
+            if (
+                !result ||
+                typeof result !== 'object' ||
+                !('revoked' in result) ||
+                result.revoked !== true
+            ) {
+                throw new Error('Connect cloud key revocation was not confirmed');
+            }
+        }
+    }
+
     getExtraOrigins(): string[] {
         const extraOrigins = this.configService.get<string>('store.config.api.extraOrigins');
         if (extraOrigins) {
@@ -40,18 +113,51 @@ export class ConnectConfigService {
      * This is used when the user logs out.
      * It retains the existing config, but resets identity-related fields.
      */
-    resetUser() {
+    async resetUser() {
         // overwrite identity fields, but retain destructured fields
-        const { wanaccess, wanport, upnpEnabled, ...identity } = emptyMyServersConfig();
-        this.configService.set(this.configKey, {
-            ...this.getConfig(),
+        const { wanaccess, wanport, upnpEnabled, ...identity } = {
+            ...emptyMyServersConfig(),
+            email: '',
+        };
+        const previousConfig = this.getConfig();
+        const config = {
+            ...previousConfig,
             ...identity,
-        });
+        };
+        const persister = new ConnectConfigPersister(this.configService);
+        this.configService.set(this.configKey, config);
+        let writeCompleted = false;
+        try {
+            writeCompleted = await persister.persist(config);
+            const savedConfig = await persister.getFileHandler().readConfigFile();
+            if (!isMatch(savedConfig, identity)) {
+                throw new Error('Connect sign-out could not persist cleared identity');
+            }
+        } catch (error) {
+            if (!writeCompleted) {
+                this.configService.set(this.configKey, previousConfig);
+                throw error;
+            }
+            // The disk may already contain the cleared identity. Keep memory
+            // cleared so shutdown persistence cannot restore the revoked key.
+            throw new ConnectIdentityVerificationError(error);
+        }
+
+        const legacyPath = this.configService.get<string>(
+            'PATHS_MY_SERVERS_CONFIG',
+            '/boot/config/plugins/dynamix.my.servers/myservers.cfg'
+        );
+        if (legacyPath) {
+            await unlink(legacyPath).catch((error: NodeJS.ErrnoException) => {
+                if (error.code !== 'ENOENT') throw new ConnectLegacyCleanupError(error);
+            });
+        }
         this.logger.verbose('Reset Connect user identity');
     }
 
     @OnEvent(EVENTS.LOGOUT, { async: true })
-    async onLogout() {
-        this.resetUser();
+    async onLogout({ configResetCompleted = false }: { configResetCompleted?: boolean } = {}) {
+        if (configResetCompleted) return;
+        await this.resetUser();
     }
 }

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.events.ts

@@ -30,10 +30,7 @@ export class MothershipHandler {
     @OnEvent(EVENTS.MOTHERSHIP_CONNECTION_STATUS_CHANGED, { async: true })
     async onMothershipConnectionStatusChanged() {
         const state = this.connectionService.getConnectionState();
-        if (
-            state &&
-            [MinigraphStatus.PING_FAILURE].includes(state.status)
-        ) {
+        if (state && [MinigraphStatus.PING_FAILURE].includes(state.status)) {
             this.logger.verbose(
                 'Mothership connection status changed to %s; setting up mothership subscription',
                 state.status
@@ -52,11 +49,13 @@ export class MothershipHandler {
     @OnEvent(EVENTS.LOGOUT, { async: true, prependListener: true })
     async logout({ reason }: { reason?: string }) {
         this.logger.log('Logging out user: %s', reason ?? 'No reason provided');
-        // publish to the 'servers' and 'owner' endpoints
-        await this.legacyPubSub.publish(GRAPHQL_PUBSUB_CHANNEL.SERVERS, { servers: [] });
-        await this.legacyPubSub.publish(GRAPHQL_PUBSUB_CHANNEL.OWNER, {
-            owner: { username: 'root', url: '', avatar: '' },
-        });
-        await this.mothershipController.stop();
+        try {
+            await this.mothershipController.stop();
+        } finally {
+            await this.legacyPubSub.publish(GRAPHQL_PUBSUB_CHANNEL.SERVERS, { servers: [] });
+            await this.legacyPubSub.publish(GRAPHQL_PUBSUB_CHANNEL.OWNER, {
+                owner: { username: 'root', url: '', avatar: '' },
+            });
+        }
     }
 }

packages/unraid-api-plugin-connect/src/unraid-connect/connect-settings.resolver.ts

@@ -9,6 +9,11 @@ import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js';
 import { UsePermissions } from '@unraid/shared/use-permissions.directive.js';
 import { GraphQLJSON } from 'graphql-scalars';
 
+import {
+    ConnectConfigService,
+    ConnectIdentityVerificationError,
+    ConnectLegacyCleanupError,
+} from '../config/connect.config.service.js';
 import { EVENTS } from '../helper/nest-tokens.js';
 import { ConnectSettingsService } from './connect-settings.service.js';
 import {
@@ -28,7 +33,8 @@ export class ConnectSettingsResolver {
 
     constructor(
         private readonly connectSettingsService: ConnectSettingsService,
-        private readonly eventEmitter: EventEmitter2
+        private readonly eventEmitter: EventEmitter2,
+        private readonly connectConfigService: ConnectConfigService
     ) {}
 
     @UsePermissions({
@@ -118,7 +124,31 @@ export class ConnectSettingsResolver {
         resource: Resource.CONNECT,
     })
     public async connectSignOut() {
-        this.eventEmitter.emit(EVENTS.LOGOUT, { reason: 'Manual Sign Out Using API' });
+        await this.connectConfigService.revokeCloudKey();
+        try {
+            await this.connectConfigService.resetUser();
+        } catch (error) {
+            if (
+                error instanceof ConnectLegacyCleanupError ||
+                error instanceof ConnectIdentityVerificationError
+            ) {
+                // The write completed, so keep the identity cleared and tear
+                // down the relay even if cleanup or verification failed.
+                try {
+                    await this.eventEmitter.emitAsync(EVENTS.LOGOUT, {
+                        reason: 'Manual Sign Out Using API',
+                        configResetCompleted: true,
+                    });
+                } finally {
+                    throw error;
+                }
+            }
+            throw error;
+        }
+        await this.eventEmitter.emitAsync(EVENTS.LOGOUT, {
+            reason: 'Manual Sign Out Using API',
+            configResetCompleted: true,
+        });
         return true;
     }