diff --git a/api/src/core/utils/clients/emcmd.spec.ts b/api/src/core/utils/clients/emcmd.spec.ts new file mode 100644 index 0000000000..d1dfac5ca9 --- /dev/null +++ b/api/src/core/utils/clients/emcmd.spec.ts @@ -0,0 +1,111 @@ +import { readFile } from 'node:fs/promises'; + +import { got } from 'got'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { emcmd } from '@app/core/utils/clients/emcmd.js'; +import { getters, store } from '@app/store/index.js'; + +vi.mock('node:fs/promises', async () => { + const actual = await vi.importActual('node:fs/promises'); + return { + ...actual, + readFile: vi.fn(), + }; +}); + +vi.mock('got', () => ({ + got: { + post: vi.fn(), + }, +})); + +vi.mock('@app/store/index.js', () => ({ + getters: { + emhttp: vi.fn(), + paths: vi.fn(), + }, + store: { + dispatch: vi.fn(), + }, +})); + +describe('emcmd', () => { + beforeEach(() => { + vi.clearAllMocks(); + + vi.mocked(getters.paths).mockReturnValue({ + 'emhttpd-socket': '/var/run/emhttpd.socket', + } as ReturnType); + + vi.mocked(getters.emhttp).mockReturnValue({ + var: { + csrfToken: 'state-token', + }, + } as ReturnType); + + vi.mocked(got.post).mockResolvedValue({ + body: '', + statusCode: 200, + } as Awaited>); + }); + + it('uses got over the emhttp unix socket', async () => { + const result = await emcmd({ + changeNames: 'Apply', + NAME: 'Hello', + }); + + expect(got.post).toHaveBeenCalledWith('http://unix:/var/run/emhttpd.socket:/update', { + enableUnixSockets: true, + body: 'changeNames=Apply&NAME=Hello&csrf_token=state-token', + headers: { + 'content-type': 'application/x-www-form-urlencoded', + }, + throwHttpErrors: false, + }); + + expect(result).toMatchObject({ + body: '', + statusCode: 200, + }); + }); + + it('throws emhttp output when got returns a non-empty body', async () => { + vi.mocked(got.post).mockResolvedValue({ + body: '', + statusCode: 200, + } as Awaited>); + + await expect(emcmd({ changeNames: 'Apply' })).rejects.toThrow( + '' + ); + }); + + it('throws on http failures reported by got', async () => { + vi.mocked(got.post).mockResolvedValue({ + body: '', + statusCode: 500, + } as Awaited>); + + await expect(emcmd({ changeNames: 'Apply' })).rejects.toThrow(); + }); + + it('falls back to var.ini for the csrf token before retrying state loads', async () => { + vi.mocked(getters.emhttp).mockReturnValue({ + var: {}, + } as ReturnType); + vi.mocked(readFile).mockResolvedValue('csrf_token="ini-token"\n'); + + await emcmd({ changeNames: 'Apply' }); + + expect(readFile).toHaveBeenCalledWith('/var/local/emhttp/var.ini', 'utf-8'); + expect(store.dispatch).not.toHaveBeenCalled(); + expect(got.post).toHaveBeenCalledWith( + 'http://unix:/var/run/emhttpd.socket:/update', + expect.objectContaining({ + body: 'changeNames=Apply&csrf_token=ini-token', + }) + ); + }); +}); diff --git a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts index a2e1a134ff..56f98d4b5d 100644 --- a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts @@ -802,7 +802,7 @@ describe('OnboardingService', () => { }); // --- Spy on subsequent steps to ensure they are still called --- - // We already mock fs.writeFile, so we can check calls to userDynamixCfg and identCfg + // We already mock fs.writeFile, so we can check calls to userDynamixCfg const applyDisplaySettingsSpy = vi.spyOn(service as any, 'applyDisplaySettings'); const updateCfgFileSpy = vi.spyOn(service as any, 'updateCfgFile'); @@ -978,7 +978,6 @@ describe('OnboardingService', () => { (service as any).activationDir = activationDir; (service as any).configFile = userDynamixCfg; (service as any).caseModelCfg = caseModelCfg; - (service as any).identCfg = identCfg; (service as any).activationData = plainToInstance(ActivationCode, { ...mockActivationData }); (service as any).activationJsonPath = activationJsonPath; (service as any).materializedPartnerMedia = { @@ -988,7 +987,6 @@ describe('OnboardingService', () => { // Mock necessary file reads/writes vi.mocked(fs.readFile).mockImplementation(async (p) => { if (p === userDynamixCfg) return ini.stringify({ display: { existing: 'value' } }); - if (p === identCfg) return ini.stringify({ NAME: 'OldName' }); if (p === caseModelCfg) return 'old-model.png'; // Simulate file not found for updateCfgFile tests where it matters // If activation JSON is read here, return mock data @@ -1361,353 +1359,6 @@ describe('OnboardingService', () => { expect(fs.writeFile).toHaveBeenCalledWith(caseModelCfg, 'mid-tower'); expect(loggerLogSpy).toHaveBeenCalledWith(`Case model set to mid-tower in ${caseModelCfg}`); }); - - it('applyServerIdentity should call emcmd directly', async () => { - const updateSpy = vi.spyOn(service as any, 'updateCfgFile'); - - // Capture the emcmd call parameters - let emcmdParams; - vi.mocked(emcmd).mockImplementation(async (params, options) => { - emcmdParams = { params, options }; - return { body: '', ok: true } as any; - }); - - const promise = (service as any).applyServerIdentity(); - await vi.runAllTimers(); - await promise; - - // We no longer update the config file before calling emcmd - expect(updateSpy).not.toHaveBeenCalled(); - - // Verify emcmd was called with expected parameters using inline snapshot - expect(emcmdParams).toMatchInlineSnapshot(` - { - "options": { - "waitForToken": true, - }, - "params": { - "COMMENT": "Partner Comment", - "NAME": "PartnerServer", - "SYS_MODEL": "PartnerModel", - "changeNames": "Apply", - "server_addr": "", - "server_name": "", - }, - } - `); - - expect(loggerLogSpy).toHaveBeenCalledWith('emcmd executed successfully.'); - }, 10000); - - it('applyServerIdentity should skip if no relevant activation data', async () => { - const updateSpy = vi.spyOn(service as any, 'updateCfgFile'); - // Simulate empty DTO - (service as any).activationData = plainToInstance(ActivationCode, {}); - await (service as any).applyServerIdentity(); - expect(updateSpy).not.toHaveBeenCalled(); - expect(emcmd).not.toHaveBeenCalled(); - expect(loggerLogSpy).toHaveBeenCalledWith( - 'No server identity information found in activation data.' - ); - }); - - it('applyServerIdentity should skip if activation data has no relevant fields', async () => { - const updateSpy = vi.spyOn(service as any, 'updateCfgFile'); - // Simulate DTO with non-identity fields - (service as any).activationData = plainToInstance(ActivationCode, { - branding: { theme: 'white' }, - }); - await (service as any).applyServerIdentity(); - expect(updateSpy).not.toHaveBeenCalled(); - expect(emcmd).not.toHaveBeenCalled(); - expect(loggerLogSpy).toHaveBeenCalledWith( - 'No server identity information found in activation data.' - ); - }); - - it('applyServerIdentity should log error on emcmd failure', async () => { - const emcmdError = new Error('Failed to call emcmd'); - - // Set up activation data directly - (service as any).activationData = plainToInstance(ActivationCode, { - system: { - serverName: 'PartnerServer', - model: 'PartnerModel', - comment: 'Partner Comment', - }, - }); - - // Mock emcmd to throw - vi.mocked(emcmd).mockRejectedValue(emcmdError); - - // Clear previous log calls - loggerErrorSpy.mockClear(); - - // Call the method directly - await (service as any).applyServerIdentity(); - - // Verify the error was logged - expect(emcmd).toHaveBeenCalled(); - expect(loggerErrorSpy).toHaveBeenCalledWith( - 'Error applying server identity: %o', - emcmdError - ); - }, 10000); - - it('applyServerIdentity should apply comment even when name/model are absent', async () => { - (service as any).activationData = plainToInstance(ActivationCode, { - system: { - comment: 'Partner Comment', - }, - }); - - let commentOnlyParams: Record | undefined; - vi.mocked(emcmd).mockImplementation(async (params) => { - commentOnlyParams = params as Record; - return { body: '', ok: true } as any; - }); - - await (service as any).applyServerIdentity(); - - expect(emcmd).toHaveBeenCalled(); - expect(commentOnlyParams).toMatchObject({ - COMMENT: 'Partner Comment', - changeNames: 'Apply', - server_addr: '', - server_name: '', - }); - expect(commentOnlyParams).not.toHaveProperty('NAME'); - expect(commentOnlyParams).not.toHaveProperty('SYS_MODEL'); - }); - - it('applyServerIdentity should omit comment when activation data does not provide one', async () => { - (service as any).activationData = plainToInstance(ActivationCode, { - system: { - serverName: 'PartnerServer', - model: 'PartnerModel', - }, - }); - - let paramsWithoutComment: Record | undefined; - vi.mocked(emcmd).mockImplementation(async (params) => { - paramsWithoutComment = params as Record; - return { body: '', ok: true } as any; - }); - - await (service as any).applyServerIdentity(); - - expect(emcmd).toHaveBeenCalled(); - expect(paramsWithoutComment).toMatchObject({ - NAME: 'PartnerServer', - SYS_MODEL: 'PartnerModel', - }); - expect(paramsWithoutComment).not.toHaveProperty('COMMENT'); - }); - - it('applyServerIdentity should allow explicitly empty comments from activation data', async () => { - (service as any).activationData = plainToInstance(ActivationCode, { - system: { - comment: '', - }, - }); - - let emptyCommentParams: Record | undefined; - vi.mocked(emcmd).mockImplementation(async (params) => { - emptyCommentParams = params as Record; - return { body: '', ok: true } as any; - }); - - await (service as any).applyServerIdentity(); - - expect(emcmd).toHaveBeenCalled(); - expect(emptyCommentParams).toMatchObject({ - COMMENT: '', - changeNames: 'Apply', - server_addr: '', - server_name: '', - }); - }); - - it.each([ - { - caseName: 'name only', - system: { serverName: 'PartnerServer' }, - expected: { NAME: 'PartnerServer' }, - omitted: ['SYS_MODEL', 'COMMENT'], - }, - { - caseName: 'model only', - system: { model: 'PartnerModel' }, - expected: { SYS_MODEL: 'PartnerModel' }, - omitted: ['NAME', 'COMMENT'], - }, - { - caseName: 'comment only', - system: { comment: 'Partner Comment' }, - expected: { COMMENT: 'Partner Comment' }, - omitted: ['NAME', 'SYS_MODEL'], - }, - { - caseName: 'explicit empty comment', - system: { comment: '' }, - expected: { COMMENT: '' }, - omitted: ['NAME', 'SYS_MODEL'], - }, - ])( - 'applyServerIdentity should map partial identity fields correctly ($caseName)', - async (scenario) => { - (service as any).activationData = plainToInstance(ActivationCode, { - system: scenario.system, - }); - - let params: Record | undefined; - vi.mocked(emcmd).mockImplementation(async (incomingParams) => { - params = incomingParams as Record; - return { body: '', ok: true } as any; - }); - - await (service as any).applyServerIdentity(); - - expect(emcmd).toHaveBeenCalledTimes(1); - expect(params).toMatchObject({ - ...scenario.expected, - changeNames: 'Apply', - server_addr: '', - server_name: '', - }); - scenario.omitted.forEach((key) => { - expect(params).not.toHaveProperty(key); - }); - } - ); - - it('applyServerIdentity should truncate serverName if too long', async () => { - const longServerName = 'ThisServerNameIsWayTooLongForUnraid'; // Length > 16 - const truncatedServerName = longServerName.slice(0, 15); // Expected truncated length - // Simulate DTO with long serverName after plainToClass - - const testActivationParser = await plainToInstance(ActivationCode, { - ...mockActivationData, - system: { ...mockActivationData.system, serverName: longServerName }, - }); - - expect(testActivationParser.system?.serverName).toBe(truncatedServerName); - }); - - it('applyServerIdentity should sanitize and truncate activation comments', async () => { - const unsafeLongComment = `${'"\\'.repeat(40)}${'A'.repeat(100)}`; - const parsedActivation = plainToInstance(ActivationCode, { - system: { - comment: unsafeLongComment, - }, - }); - - expect(parsedActivation.system?.comment).toBeDefined(); - expect(parsedActivation.system?.comment).not.toMatch(/["\\]/); - expect(parsedActivation.system?.comment!.length).toBeLessThanOrEqual(64); - }); - - it('applyServerIdentity should send sanitized identity values from transformed activation data', async () => { - const unsafeIdentity = plainToInstance(ActivationCode, { - system: { - serverName: 'Par"t\\nerServer', - model: 'Pa"rt\\nerModel', - comment: 'Partn"er\\Comment', - }, - }); - (service as any).activationData = unsafeIdentity; - - let params: Record | undefined; - vi.mocked(emcmd).mockImplementation(async (incomingParams) => { - params = incomingParams as Record; - return { body: '', ok: true } as any; - }); - - await (service as any).applyServerIdentity(); - - expect(emcmd).toHaveBeenCalledTimes(1); - expect(params).toMatchObject({ - NAME: 'PartnerServer', - SYS_MODEL: 'PartnerModel', - COMMENT: 'PartnerComment', - }); - expect(params?.NAME).not.toMatch(/["\\]/); - expect(params?.SYS_MODEL).not.toMatch(/["\\]/); - expect(params?.COMMENT).not.toMatch(/["\\]/); - }); - - it('should correctly pass server_https parameter based on nginx state', async () => { - // Mock getters.emhttp to include nginx with sslEnabled=true - const mockEmhttpWithSsl = { - nginx: { sslEnabled: true }, - var: { name: 'Tower', sysModel: 'Custom', comment: 'Default' }, - }; - vi.mocked(getters.emhttp).mockReturnValue(mockEmhttpWithSsl as any); - - // Set up the service's activationData field directly - (service as any).activationData = plainToInstance(ActivationCode, { - system: { - serverName: 'PartnerServer', - model: 'PartnerModel', - comment: 'Partner Comment', - }, - }); - - // Mock emcmd and capture the params for snapshot testing - let sslEnabledParams; - vi.mocked(emcmd).mockImplementation(async (params) => { - sslEnabledParams = params; - return { body: '', ok: true } as any; - }); - - // Call the method directly to test SSL enabled case - await (service as any).applyServerIdentity(); - - // Verify emcmd was called - expect(emcmd).toHaveBeenCalled(); - // Use toMatchInlineSnapshot to compare the params - expect(sslEnabledParams).toMatchInlineSnapshot(` - { - "COMMENT": "Partner Comment", - "NAME": "PartnerServer", - "SYS_MODEL": "PartnerModel", - "changeNames": "Apply", - "server_addr": "", - "server_name": "", - } - `); - - // Now test with SSL disabled - const mockEmhttpNoSsl = { - nginx: { sslEnabled: false }, - var: { name: 'Tower', sysModel: 'Custom', comment: 'Default' }, - }; - vi.mocked(getters.emhttp).mockReturnValue(mockEmhttpNoSsl as any); - - // Update the mock to capture params for the second call - let sslDisabledParams; - vi.mocked(emcmd).mockImplementation(async (params) => { - sslDisabledParams = params; - return { body: '', ok: true } as any; - }); - - // Call again to test SSL disabled case - await (service as any).applyServerIdentity(); - - // Verify emcmd was called again - expect(emcmd).toHaveBeenCalled(); - // Use toMatchInlineSnapshot to compare the params - expect(sslDisabledParams).toMatchInlineSnapshot(` - { - "COMMENT": "Partner Comment", - "NAME": "PartnerServer", - "SYS_MODEL": "PartnerModel", - "changeNames": "Apply", - "server_addr": "", - "server_name": "", - } - `); - }, 10000); }); }); @@ -1722,7 +1373,6 @@ describe('applyActivationCustomizations specific tests', () => { const activationDir = mockPaths.activationBase; const userDynamixCfg = mockPaths['dynamix-config'][1]; const caseModelCfg = mockPaths.boot.caseModelConfig; - const identCfg = mockPaths.identConfig; const bannerSource = mockPaths.activation.banner; const bannerTarget = mockPaths.webgui.banner; const caseModelSource = mockPaths.activation.caseModel; @@ -1787,7 +1437,6 @@ describe('applyActivationCustomizations specific tests', () => { (service as any).activationDir = activationDir; (service as any).configFile = userDynamixCfg; (service as any).caseModelCfg = caseModelCfg; - (service as any).identCfg = identCfg; (service as any).activationData = plainToInstance(ActivationCode, { ...mockActivationData }); // Default mocks for dependencies, override in specific tests if needed @@ -1797,7 +1446,6 @@ describe('applyActivationCustomizations specific tests', () => { vi.mocked(fs.access).mockResolvedValue(undefined); // Assume dirs/files accessible by default vi.mocked(fs.readFile).mockImplementation(async (p) => { if (p === userDynamixCfg) return ini.stringify({}); - if (p === identCfg) return ini.stringify({}); if (p === caseModelCfg) return ''; // Assume empty or non-existent return ''; }); diff --git a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts index 1b22ed2c11..1d08bea764 100644 --- a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts +++ b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts @@ -9,7 +9,6 @@ import * as ini from 'ini'; import coerce from 'semver/functions/coerce.js'; import gte from 'semver/functions/gte.js'; -import { emcmd } from '@app/core/utils/clients/emcmd.js'; import { fileExists } from '@app/core/utils/files/file-exists.js'; import { safelySerializeObjectToIni } from '@app/core/utils/files/safe-ini-serializer.js'; import { loadDynamixConfigFromDiskSync } from '@app/store/actions/load-dynamix-config-file.js'; @@ -43,7 +42,6 @@ export class OnboardingService implements OnModuleInit { private activationDir!: string; private configFile!: string; private caseModelCfg!: string; - private identCfg!: string; private activationJsonPath: string | null = null; private materializedPartnerMedia: Record<'banner' | 'caseModel', boolean> = { banner: false, @@ -76,7 +74,6 @@ export class OnboardingService implements OnModuleInit { this.activationDir = paths.activationBase; this.configFile = paths['dynamix-config']?.[1]; this.caseModelCfg = paths.boot?.caseModelConfig; - this.identCfg = paths.identConfig; this.logger.log('OnboardingService initialized with paths from store.'); @@ -917,59 +914,6 @@ export class OnboardingService implements OnModuleInit { } } - private async applyServerIdentity() { - if (!this.activationData) { - this.logger.warn('No activation data available for server identity setup.'); - return; - } - - this.logger.log('Applying server identity...'); - // Ideally, get current values from Redux store instead of var.ini - // Assuming EmhttpState type provides structure for emhttp slice. Adjust if necessary. - // Using optional chaining ?. in case emhttp or var is not defined in the state yet. - const currentEmhttpState = getters.emhttp(); - const currentName = currentEmhttpState?.var?.name || ''; - // Skip sending sysModel to emcmd for now - const currentSysModel = ''; - const currentComment = currentEmhttpState?.var?.comment || ''; - - this.logger.debug( - `Current identity - Name: ${currentName}, Model: ${currentSysModel}, Comment: ${currentComment}` - ); - - const { serverName, model: sysModel, comment } = this.activationData.system || {}; - const paramsToUpdate: Record = { - ...(serverName && { NAME: serverName }), - ...(sysModel && { SYS_MODEL: sysModel }), - ...(comment !== undefined && { COMMENT: comment }), - }; - - if (Object.keys(paramsToUpdate).length === 0) { - this.logger.log('No server identity information found in activation data.'); - return; - } - - this.logger.log('Updating server identity:', paramsToUpdate); - - try { - // Trigger emhttp update via emcmd - const updateParams = { - ...paramsToUpdate, - changeNames: 'Apply', - // Can be null string - server_name: '', - // Can be null string - server_addr: '', - }; - this.logger.log(`Calling emcmd with params: %o`, updateParams); - await emcmd(updateParams, { waitForToken: true }); - - this.logger.log('emcmd executed successfully.'); - } catch (error: unknown) { - this.logger.error('Error applying server identity: %o', error); - } - } - // Helper function to update .cfg files (like dynamix.cfg or ident.cfg) using the ini library private async updateCfgFile( filePath: string, diff --git a/api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts b/api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts index b24bcd2455..2fc2f1e287 100644 --- a/api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts @@ -1,8 +1,15 @@ +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + import { GraphQLError } from 'graphql'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { emcmd } from '@app/core/utils/clients/emcmd.js'; import { getters } from '@app/store/index.js'; +import { type SliceState } from '@app/store/modules/emhttp.js'; +import { FileLoadStatus } from '@app/store/types.js'; +import { ArrayState } from '@app/unraid-api/graph/resolvers/array/array.model.js'; import { ServerService } from '@app/unraid-api/graph/resolvers/servers/server.service.js'; vi.mock('@app/core/utils/clients/emcmd.js', () => ({ @@ -12,27 +19,84 @@ vi.mock('@app/core/utils/clients/emcmd.js', () => ({ vi.mock('@app/store/index.js', () => ({ getters: { emhttp: vi.fn(), + paths: vi.fn(), }, })); +const createEmhttpState = ({ + name = 'Tower', + comment = 'Tower comment', + sysModel = 'Model X100', + fsState = 'Stopped', + mdState, + sslEnabled = true, +}: { + name?: string; + comment?: string; + sysModel?: string; + fsState?: string; + mdState?: SliceState['var']['mdState']; + sslEnabled?: boolean; +} = {}): SliceState => ({ + status: FileLoadStatus.LOADED, + var: { + name, + comment, + sysModel, + fsState, + mdState, + regGuid: 'GUID-123', + port: 80, + } as unknown as SliceState['var'], + devices: [], + networks: [{ ipaddr: ['192.168.1.10'] }] as unknown as SliceState['networks'], + nginx: { + sslEnabled, + lanName: 'tower.local', + lanIp: '192.168.1.10', + } as unknown as SliceState['nginx'], + shares: [], + disks: [], + users: [], + smbShares: [], + nfsShares: [], +}); + describe('ServerService', () => { let service: ServerService; + let tempDirectory: string; + let identConfigPath: string; - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks(); service = new ServerService(); + tempDirectory = await mkdtemp(join(tmpdir(), 'server-service-')); + identConfigPath = join(tempDirectory, 'boot/config/ident.cfg'); - vi.mocked(getters.emhttp).mockReturnValue({ - var: { - name: 'Tower', - fsState: 'Stopped', - regGuid: 'GUID-123', - port: '80', - comment: 'Tower comment', - }, - networks: [{ ipaddr: ['192.168.1.10'] }], - } as unknown as ReturnType); - vi.mocked(emcmd).mockResolvedValue({ ok: true } as Awaited>); + vi.mocked(getters.emhttp).mockReturnValue(createEmhttpState()); + vi.mocked(getters.paths).mockReturnValue({ + identConfig: identConfigPath, + } as ReturnType); + + await mkdir(join(tempDirectory, 'boot/config'), { recursive: true }); + await writeFile( + identConfigPath, + 'NAME="Tower"\nCOMMENT="Tower comment"\nSYS_MODEL="Model X100"\nEXTRA="keep-me"\n', + 'utf8' + ); + + vi.mocked(emcmd).mockImplementation(async (params) => { + await writeFile( + identConfigPath, + `NAME="${params.NAME}"\nCOMMENT="${params.COMMENT}"\nSYS_MODEL="${params.SYS_MODEL}"\nEXTRA="keep-me"\n`, + 'utf8' + ); + return { ok: true } as Awaited>; + }); + }); + + afterEach(async () => { + await rm(tempDirectory, { recursive: true, force: true }); }); it('throws for invalid server name characters', async () => { @@ -82,13 +146,12 @@ describe('ServerService', () => { }); it('requires stopped array only when name changes', async () => { - vi.mocked(getters.emhttp).mockReturnValue({ - var: { - name: 'Tower', - mdState: 'STARTED', + vi.mocked(getters.emhttp).mockReturnValue( + createEmhttpState({ fsState: 'Started', - }, - } as any); + mdState: ArrayState.STARTED, + }) + ); await expect(service.updateServerIdentity('NewTower', 'desc')).rejects.toThrow( 'The array must be stopped to change the server name.' @@ -100,36 +163,50 @@ describe('ServerService', () => { }); }); - it('allows name change when mdState is STOPPED even if fsState is not Stopped (internal boot)', async () => { - vi.mocked(getters.emhttp).mockReturnValue({ - var: { - name: 'Tower', - mdState: 'STOPPED', - fsState: 'Started', - regGuid: 'GUID-123', - port: '80', + it('allows name change when mdState is STOPPED even if fsState is not Stopped', async () => { + vi.mocked(getters.emhttp).mockReturnValue( + createEmhttpState({ comment: '', - }, - networks: [{ ipaddr: ['192.168.1.10'] }], - } as unknown as ReturnType); + sysModel: '', + fsState: 'Started', + mdState: ArrayState.STOPPED, + }) + ); await expect(service.updateServerIdentity('NewTower', 'desc')).resolves.toMatchObject({ name: 'NewTower', }); }); - it('calls emcmd with expected params and returns optimistic server', async () => { - const result = await service.updateServerIdentity('Tower', 'Primary host'); + it('sends the Identification.page payload and persists ident.cfg', async () => { + vi.mocked(emcmd).mockImplementation(async (params) => { + await writeFile( + identConfigPath, + `NAME="${params.NAME}"\nCOMMENT="${params.COMMENT}"\nSYS_MODEL="${params.SYS_MODEL}"\nEXTRA="keep-me"\n`, + 'utf8' + ); + return { ok: true } as Awaited>; + }); + + const result = await service.updateServerIdentity('Test1e', 'Test server1e', 'Model X200'); expect(emcmd).toHaveBeenCalledWith( { changeNames: 'Apply', - NAME: 'Tower', - COMMENT: 'Primary host', + server_https: 'on', + server_name: 'tower.local', + server_addr: '192.168.1.10', + NAME: 'Test1e', + COMMENT: 'Test server1e', + SYS_MODEL: 'Model X200', }, { waitForToken: true } ); + await expect(readFile(identConfigPath, 'utf8')).resolves.toBe( + 'NAME="Test1e"\nCOMMENT="Test server1e"\nSYS_MODEL="Model X200"\nEXTRA="keep-me"\n' + ); + expect(result).toEqual({ id: 'local', owner: { @@ -140,8 +217,8 @@ describe('ServerService', () => { }, guid: 'GUID-123', apikey: '', - name: 'Tower', - comment: 'Primary host', + name: 'Test1e', + comment: 'Test server1e', status: 'ONLINE', wanip: '', lanip: '192.168.1.10', @@ -150,10 +227,42 @@ describe('ServerService', () => { }); }); + it('preserves current comment and model when omitted, like the webgui form does', async () => { + vi.mocked(emcmd).mockImplementation(async (params) => { + await writeFile( + identConfigPath, + `NAME="${params.NAME}"\nCOMMENT="${params.COMMENT}"\nSYS_MODEL="${params.SYS_MODEL}"\nEXTRA="keep-me"\n`, + 'utf8' + ); + return { ok: true } as Awaited>; + }); + + await service.updateServerIdentity('TowerRenamed'); + + expect(emcmd).toHaveBeenCalledWith( + { + changeNames: 'Apply', + server_https: 'on', + server_name: 'tower.local', + server_addr: '192.168.1.10', + NAME: 'TowerRenamed', + COMMENT: 'Tower comment', + SYS_MODEL: 'Model X100', + }, + { waitForToken: true } + ); + + await expect(readFile(identConfigPath, 'utf8')).resolves.toBe( + 'NAME="TowerRenamed"\nCOMMENT="Tower comment"\nSYS_MODEL="Model X100"\nEXTRA="keep-me"\n' + ); + }); + it('skips emcmd when identity values are unchanged', async () => { - const result = await service.updateServerIdentity('Tower', 'Tower comment'); + const before = await readFile(identConfigPath, 'utf8'); + const result = await service.updateServerIdentity('Tower', 'Tower comment', 'Model X100'); expect(emcmd).not.toHaveBeenCalled(); + await expect(readFile(identConfigPath, 'utf8')).resolves.toBe(before); expect(result).toMatchObject({ name: 'Tower', comment: 'Tower comment', @@ -161,43 +270,79 @@ describe('ServerService', () => { }); }); - it('skips emcmd when sysModel is unchanged', async () => { - vi.mocked(getters.emhttp).mockReturnValue({ - var: { - name: 'Tower', - fsState: 'Stopped', - regGuid: 'GUID-123', - port: '80', - comment: 'Tower comment', - sysModel: 'Model X200', - }, - networks: [{ ipaddr: ['192.168.1.10'] }], - } as unknown as ReturnType); - - await service.updateServerIdentity('Tower', 'Tower comment', 'Model X200'); - - expect(emcmd).not.toHaveBeenCalled(); - }); + it('writes server_https as empty when ssl is disabled', async () => { + vi.mocked(getters.emhttp).mockReturnValue( + createEmhttpState({ + sslEnabled: false, + }) + ); - it('includes SYS_MODEL when provided', async () => { - await service.updateServerIdentity('Tower', 'Primary host', 'Model X200'); + await service.updateServerIdentity('Tower', 'Primary host', 'Model X100'); expect(emcmd).toHaveBeenCalledWith( - { - changeNames: 'Apply', - NAME: 'Tower', - COMMENT: 'Primary host', - SYS_MODEL: 'Model X200', - }, + expect.objectContaining({ + server_https: '', + }), { waitForToken: true } ); }); - it('throws generic failure when emcmd fails', async () => { + it('returns success when emcmd errors but ident.cfg has the requested identity', async () => { + vi.mocked(emcmd).mockImplementation(async () => { + await writeFile( + identConfigPath, + 'NAME="Tower"\nCOMMENT="Primary host"\nSYS_MODEL="Model X100"\nEXTRA="keep-me"\n', + 'utf8' + ); + throw new Error('socket failure'); + }); + + await expect(service.updateServerIdentity('Tower', 'Primary host')).resolves.toMatchObject({ + name: 'Tower', + comment: 'Primary host', + }); + }); + + it('throws generic failure when emcmd fails and ident.cfg stays unchanged', async () => { vi.mocked(emcmd).mockRejectedValue(new Error('socket failure')); + const before = await readFile(identConfigPath, 'utf8'); - await expect(service.updateServerIdentity('Tower', 'Primary host')).rejects.toThrow( - 'Failed to update server identity' - ); + await expect(service.updateServerIdentity('Tower', 'Primary host')).rejects.toMatchObject({ + message: 'Failed to update server identity', + extensions: { + cause: 'socket failure', + }, + }); + + await expect(readFile(identConfigPath, 'utf8')).resolves.toBe(before); + }); + + it('throws when emcmd succeeds but ident.cfg does not contain the requested identity', async () => { + vi.mocked(emcmd).mockResolvedValue({ ok: true } as Awaited>); + + await expect(service.updateServerIdentity('Tower', 'Primary host')).rejects.toMatchObject({ + message: 'Failed to update server identity', + extensions: { + cause: 'ident.cfg was not updated with the requested identity', + persistedIdentity: { + name: 'Tower', + comment: 'Tower comment', + sysModel: 'Model X100', + }, + }, + }); + }); + + it('throws when persisted identity cannot be verified', async () => { + vi.mocked(getters.paths).mockReturnValue({ + identConfig: join(tempDirectory, 'boot/config/missing-ident.cfg'), + } as ReturnType); + + await expect(service.updateServerIdentity('Tower', 'Primary host')).rejects.toMatchObject({ + message: 'Failed to update server identity', + extensions: { + cause: expect.stringContaining('ENOENT'), + }, + }); }); }); diff --git a/api/src/unraid-api/graph/resolvers/servers/server.service.ts b/api/src/unraid-api/graph/resolvers/servers/server.service.ts index 561d865dca..85ab7297ec 100644 --- a/api/src/unraid-api/graph/resolvers/servers/server.service.ts +++ b/api/src/unraid-api/graph/resolvers/servers/server.service.ts @@ -1,6 +1,8 @@ import { Injectable, Logger } from '@nestjs/common'; +import { readFile } from 'node:fs/promises'; import { GraphQLError } from 'graphql'; +import * as ini from 'ini'; import { emcmd } from '@app/core/utils/clients/emcmd.js'; import { getters } from '@app/store/index.js'; @@ -15,6 +17,48 @@ import { export class ServerService { private readonly logger = new Logger(ServerService.name); + private async readPersistedIdentity(): Promise<{ + name: string; + comment: string; + sysModel: string; + }> { + const identConfigPath = getters.paths().identConfig; + + if (!identConfigPath) { + throw new Error('ident.cfg path not found'); + } + + const contents = await readFile(identConfigPath, 'utf8'); + const parsed = ini.parse(contents) as { + NAME?: string; + COMMENT?: string; + SYS_MODEL?: string; + }; + + return { + name: parsed.NAME ?? '', + comment: parsed.COMMENT ?? '', + sysModel: parsed.SYS_MODEL ?? '', + }; + } + + private buildIdentityUpdateParams( + emhttpState: ReturnType, + name: string, + comment: string, + sysModel: string + ): Record { + return { + changeNames: 'Apply', + server_https: emhttpState.nginx?.sslEnabled ? 'on' : '', + server_name: emhttpState.nginx?.lanName || 'localhost', + server_addr: emhttpState.nginx?.lanIp || '127.0.0.1', + NAME: name, + COMMENT: comment, + SYS_MODEL: sysModel, + }; + } + private buildServerResponse( emhttpState: ReturnType, name: string, @@ -109,27 +153,58 @@ export class ServerService { } } - const params: Record = { - changeNames: 'Apply', - NAME: name, - }; - - if (comment !== undefined) { - params.COMMENT = comment; - } - if (sysModel !== undefined) { - params.SYS_MODEL = sysModel; - } + const params = this.buildIdentityUpdateParams(currentEmhttp, name, nextComment, nextSysModel); + let emcmdError: unknown; try { await emcmd(params, { waitForToken: true }); this.logger.log('Server identity updated successfully via emcmd.'); + } catch (error) { + emcmdError = error; + this.logger.error('emcmd reported a server identity update failure', error); + } + + try { + const persistedIdentity = await this.readPersistedIdentity(); + const persistedMatches = + persistedIdentity.name === name && + persistedIdentity.comment === nextComment && + persistedIdentity.sysModel === nextSysModel; + + if (!persistedMatches) { + throw new GraphQLError('Failed to update server identity', { + extensions: { + cause: + emcmdError instanceof Error && emcmdError.message + ? emcmdError.message + : 'ident.cfg was not updated with the requested identity', + persistedIdentity, + }, + }); + } + + if (emcmdError) { + this.logger.warn( + 'emcmd reported an error, but ident.cfg contains the requested server identity.' + ); + } + const latestEmhttp = getters.emhttp(); - const responseComment = comment ?? latestEmhttp.var?.comment ?? currentComment; - return this.buildServerResponse(latestEmhttp, name, responseComment); + return this.buildServerResponse(latestEmhttp, name, nextComment); } catch (error) { - this.logger.error('Failed to update server identity', error); - throw new GraphQLError('Failed to update server identity'); + if (error instanceof GraphQLError) { + throw error; + } + + this.logger.error('Failed to verify persisted server identity', error); + throw new GraphQLError('Failed to update server identity', { + extensions: { + cause: + error instanceof Error && error.message + ? error.message + : 'Unknown server identity persistence verification failure', + }, + }); } } }