mirror of
https://github.com/unraid/api.git
synced 2026-01-01 06:01:18 -06:00
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:
@@ -1,5 +1,5 @@
|
|||||||
{
|
{
|
||||||
"version": "4.25.2",
|
"version": "4.25.3",
|
||||||
"extraOrigins": [],
|
"extraOrigins": [],
|
||||||
"sandbox": true,
|
"sandbox": true,
|
||||||
"ssoSubIds": [],
|
"ssoSubIds": [],
|
||||||
|
|||||||
@@ -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
|
||||||
|
|
||||||
@@ -0,0 +1,6 @@
|
|||||||
|
timestamp=1730937600
|
||||||
|
event=Temperature Test
|
||||||
|
subject=Warning [UNRAID] - High disk temperature detected: 45 °C
|
||||||
|
description=Disk 1 temperature has reached 45 °C (threshold: 40 °C)<br><br>Current temperatures:<br>Parity - 32 °C [OK]<br>Disk 1 - 45 °C [WARNING]<br>Disk 2 - 38 °C [OK]<br>Cache - 28 °C [OK]<br><br>Please check cooling system.
|
||||||
|
importance=warning
|
||||||
|
|
||||||
@@ -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",
|
||||||
|
|||||||
@@ -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',
|
||||||
|
|||||||
@@ -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
8
pnpm-lock.yaml
generated
@@ -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: {}
|
||||||
|
|||||||
Reference in New Issue
Block a user