fix(api): decode html entities before parsing notifications (#1768)

so the parser does not treat them as comments.

This surfaces a new bug: `#`'s in notification subject or descriptions
are treated as comments, and content following a `#` will not be
displayed in queries from the api, unless the values are explicitly
quoted as strings:
```
subject=Warning #1 OS      #  Truncates after "Warning"
subject=\#1 OS             #  Backslash escape doesn't work
subject="Warning #1 OS"    #  Double quotes work!
subject='Warning #1 OS'    #  Single quotes work!
```

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Version 4.25.3

* **Improvements**
* Enhanced notification system with improved handling of special
characters and HTML-formatted content in messages.
  * Better text rendering accuracy across all notification types.

* **Chores**
  * Updated application dependencies.
  * Version bumped to 4.25.3.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Pujit Mehrotra
2025-11-07 14:49:22 -05:00
committed by GitHub
parent 11d2de5d08
commit 42406e795d
7 changed files with 85 additions and 76 deletions

View File

@@ -1,5 +1,5 @@
{ {
"version": "4.25.2", "version": "4.25.3",
"extraOrigins": [], "extraOrigins": [],
"sandbox": true, "sandbox": true,
"ssoSubIds": [], "ssoSubIds": [],

View File

@@ -0,0 +1,6 @@
timestamp=1730937600
event=Hashtag Test
subject=Warning [UNRAID] - #1 OS is cooking
description=Disk 1 temperature has reached #epic # levels of proportion
importance=warning

View File

@@ -0,0 +1,6 @@
timestamp=1730937600
event=Temperature Test
subject=Warning [UNRAID] - High disk temperature detected: 45&#8201;&#176;C
description=Disk 1 temperature has reached 45&#8201;&#176;C (threshold: 40&#8201;&#176;C)<br><br>Current temperatures:<br>Parity - 32&#8201;&#176;C [OK]<br>Disk 1 - 45&#8201;&#176;C [WARNING]<br>Disk 2 - 38&#8201;&#176;C [OK]<br>Cache - 28&#8201;&#176;C [OK]<br><br>Please check cooling system.
importance=warning

View File

@@ -116,6 +116,7 @@
"graphql-subscriptions": "3.0.0", "graphql-subscriptions": "3.0.0",
"graphql-tag": "2.12.6", "graphql-tag": "2.12.6",
"graphql-ws": "6.0.6", "graphql-ws": "6.0.6",
"html-entities": "^2.6.0",
"ini": "5.0.0", "ini": "5.0.0",
"ip": "2.0.1", "ip": "2.0.1",
"jose": "6.0.13", "jose": "6.0.13",

View File

@@ -14,6 +14,16 @@ import {
} from '@app/unraid-api/graph/resolvers/notifications/notifications.model.js'; } from '@app/unraid-api/graph/resolvers/notifications/notifications.model.js';
import { NotificationsService } from '@app/unraid-api/graph/resolvers/notifications/notifications.service.js'; import { NotificationsService } from '@app/unraid-api/graph/resolvers/notifications/notifications.service.js';
// Mock fs/promises for unit tests
vi.mock('fs/promises', async () => {
const actual = await vi.importActual<typeof import('fs/promises')>('fs/promises');
const mockReadFile = vi.fn();
return {
...actual,
readFile: mockReadFile,
};
});
// Mock getters.dynamix, Logger, and pubsub // Mock getters.dynamix, Logger, and pubsub
vi.mock('@app/store/index.js', () => { vi.mock('@app/store/index.js', () => {
// Create test directory path inside factory function // Create test directory path inside factory function
@@ -61,24 +71,24 @@ const testNotificationsDir = join(tmpdir(), 'unraid-api-test-notifications');
describe('NotificationsService - loadNotificationFile (minimal mocks)', () => { describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
let service: NotificationsService; let service: NotificationsService;
let mockReadFile: any;
beforeEach(() => { beforeEach(async () => {
const fsPromises = await import('fs/promises');
mockReadFile = fsPromises.readFile as any;
vi.mocked(mockReadFile).mockClear();
service = new NotificationsService(); service = new NotificationsService();
}); });
it('should load and validate a valid notification file', async () => { it('should load and validate a valid notification file', async () => {
const mockNotificationIni: NotificationIni = { const mockFileContent = `timestamp=1609459200
timestamp: '1609459200', event=Test Event
event: 'Test Event', subject=Test Subject
subject: 'Test Subject', description=Test Description
description: 'Test Description', importance=alert
importance: 'alert', link=http://example.com`;
link: 'http://example.com',
};
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue( vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
mockNotificationIni
);
const result = await (service as any).loadNotificationFile( const result = await (service as any).loadNotificationFile(
'/test/path/test.notify', '/test/path/test.notify',
@@ -99,17 +109,12 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
}); });
it('should return masked warning notification on validation error (missing required fields)', async () => { it('should return masked warning notification on validation error (missing required fields)', async () => {
const invalidNotificationIni: Omit<NotificationIni, 'event'> = { const mockFileContent = `timestamp=1609459200
timestamp: '1609459200', subject=Test Subject
// event: 'Missing Event', // missing required field description=Test Description
subject: 'Test Subject', importance=alert`;
description: 'Test Description',
importance: 'alert',
};
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue( vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
invalidNotificationIni
);
const result = await (service as any).loadNotificationFile( const result = await (service as any).loadNotificationFile(
'/test/path/invalid.notify', '/test/path/invalid.notify',
@@ -121,17 +126,13 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
}); });
it('should handle invalid enum values', async () => { it('should handle invalid enum values', async () => {
const invalidNotificationIni: NotificationIni = { const mockFileContent = `timestamp=1609459200
timestamp: '1609459200', event=Test Event
event: 'Test Event', subject=Test Subject
subject: 'Test Subject', description=Test Description
description: 'Test Description', importance=not-a-valid-enum`;
importance: 'not-a-valid-enum' as any,
};
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue( vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
invalidNotificationIni
);
const result = await (service as any).loadNotificationFile( const result = await (service as any).loadNotificationFile(
'/test/path/invalid-enum.notify', '/test/path/invalid-enum.notify',
@@ -145,16 +146,12 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
}); });
it('should handle missing description field (should return masked warning notification)', async () => { it('should handle missing description field (should return masked warning notification)', async () => {
const mockNotificationIni: Omit<NotificationIni, 'description'> = { const mockFileContent = `timestamp=1609459200
timestamp: '1609459200', event=Test Event
event: 'Test Event', subject=Test Subject
subject: 'Test Subject', importance=normal`;
importance: 'normal',
};
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue( vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
mockNotificationIni
);
const result = await (service as any).loadNotificationFile( const result = await (service as any).loadNotificationFile(
'/test/path/test.notify', '/test/path/test.notify',
@@ -166,19 +163,15 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
}); });
it('should preserve passthrough data from notification file (only known fields)', async () => { it('should preserve passthrough data from notification file (only known fields)', async () => {
const mockNotificationIni: NotificationIni & { customField: string } = { const mockFileContent = `timestamp=1609459200
timestamp: '1609459200', event=Test Event
event: 'Test Event', subject=Test Subject
subject: 'Test Subject', description=Test Description
description: 'Test Description', importance=normal
importance: 'normal', link=http://example.com
link: 'http://example.com', customField=custom value`;
customField: 'custom value',
};
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue( vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
mockNotificationIni
);
const result = await (service as any).loadNotificationFile( const result = await (service as any).loadNotificationFile(
'/test/path/test.notify', '/test/path/test.notify',
@@ -201,17 +194,12 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
}); });
it('should handle missing timestamp field gracefully', async () => { it('should handle missing timestamp field gracefully', async () => {
const mockNotificationIni: Omit<NotificationIni, 'timestamp'> = { const mockFileContent = `event=Test Event
// timestamp is missing subject=Test Subject
event: 'Test Event', description=Test Description
subject: 'Test Subject', importance=alert`;
description: 'Test Description',
importance: 'alert',
};
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue( vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
mockNotificationIni
);
const result = await (service as any).loadNotificationFile( const result = await (service as any).loadNotificationFile(
'/test/path/missing-timestamp.notify', '/test/path/missing-timestamp.notify',
@@ -225,17 +213,13 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
}); });
it('should handle malformed timestamp field gracefully', async () => { it('should handle malformed timestamp field gracefully', async () => {
const mockNotificationIni: NotificationIni = { const mockFileContent = `timestamp=not-a-timestamp
timestamp: 'not-a-timestamp', event=Test Event
event: 'Test Event', subject=Test Subject
subject: 'Test Subject', description=Test Description
description: 'Test Description', importance=alert`;
importance: 'alert',
};
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue( vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
mockNotificationIni
);
const result = await (service as any).loadNotificationFile( const result = await (service as any).loadNotificationFile(
'/test/path/malformed-timestamp.notify', '/test/path/malformed-timestamp.notify',

View File

@@ -1,5 +1,5 @@
import { Injectable, Logger } from '@nestjs/common'; import { Injectable, Logger } from '@nestjs/common';
import { readdir, rename, stat, unlink, writeFile } from 'fs/promises'; import { readdir, readFile, rename, stat, unlink, writeFile } from 'fs/promises';
import { basename, join } from 'path'; import { basename, join } from 'path';
import type { Stats } from 'fs'; import type { Stats } from 'fs';
@@ -7,6 +7,7 @@ import { FSWatcher, watch } from 'chokidar';
import { ValidationError } from 'class-validator'; import { ValidationError } from 'class-validator';
import { execa } from 'execa'; import { execa } from 'execa';
import { emptyDir } from 'fs-extra'; import { emptyDir } from 'fs-extra';
import { decode } from 'html-entities';
import { encode as encodeIni } from 'ini'; import { encode as encodeIni } from 'ini';
import { v7 as uuidv7 } from 'uuid'; import { v7 as uuidv7 } from 'uuid';
@@ -648,8 +649,11 @@ export class NotificationsService {
* @throws File system errors (file not found, permission issues) or unexpected validation errors. * @throws File system errors (file not found, permission issues) or unexpected validation errors.
*/ */
private async loadNotificationFile(path: string, type: NotificationType): Promise<Notification> { private async loadNotificationFile(path: string, type: NotificationType): Promise<Notification> {
const rawContent = await readFile(path, 'utf-8');
const decodedContent = decode(rawContent);
const notificationFile = parseConfig<NotificationIni>({ const notificationFile = parseConfig<NotificationIni>({
filePath: path, file: decodedContent,
type: 'ini', type: 'ini',
}); });

8
pnpm-lock.yaml generated
View File

@@ -223,6 +223,9 @@ importers:
graphql-ws: graphql-ws:
specifier: 6.0.6 specifier: 6.0.6
version: 6.0.6(crossws@0.3.5)(graphql@16.11.0)(ws@8.18.3) version: 6.0.6(crossws@0.3.5)(graphql@16.11.0)(ws@8.18.3)
html-entities:
specifier: ^2.6.0
version: 2.6.0
ini: ini:
specifier: 5.0.0 specifier: 5.0.0
version: 5.0.0 version: 5.0.0
@@ -8264,6 +8267,9 @@ packages:
resolution: {integrity: sha512-Y22oTqIU4uuPgEemfz7NDJz6OeKf12Lsu+QC+s3BVpda64lTiMYCyGwg5ki4vFxkMwQdeZDl2adZoqUgdFuTgQ==} resolution: {integrity: sha512-Y22oTqIU4uuPgEemfz7NDJz6OeKf12Lsu+QC+s3BVpda64lTiMYCyGwg5ki4vFxkMwQdeZDl2adZoqUgdFuTgQ==}
engines: {node: '>=18'} engines: {node: '>=18'}
html-entities@2.6.0:
resolution: {integrity: sha512-kig+rMn/QOVRvr7c86gQ8lWXq+Hkv6CbAH1hLu+RG338StTpE8Z0b44SDVaqVu7HGKf27frdmUYEs9hTUX/cLQ==}
html-escaper@2.0.2: html-escaper@2.0.2:
resolution: {integrity: sha512-H2iMtd0I4Mt5eYiapRdIDjp+XzelXQ0tFE4JS7YFwFevXXMmOp9myNrUvCg0D6ws8iqkRPBfKHgbwig1SmlLfg==} resolution: {integrity: sha512-H2iMtd0I4Mt5eYiapRdIDjp+XzelXQ0tFE4JS7YFwFevXXMmOp9myNrUvCg0D6ws8iqkRPBfKHgbwig1SmlLfg==}
@@ -20667,6 +20673,8 @@ snapshots:
dependencies: dependencies:
whatwg-encoding: 3.1.1 whatwg-encoding: 3.1.1
html-entities@2.6.0: {}
html-escaper@2.0.2: {} html-escaper@2.0.2: {}
html-escaper@3.0.3: {} html-escaper@3.0.3: {}