Skip to content

Commit 42406e7

Browse files
authored
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 -->
1 parent 11d2de5 commit 42406e7

File tree

7 files changed

+92
-83
lines changed

7 files changed

+92
-83
lines changed

api/dev/configs/api.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "4.25.2",
2+
"version": "4.25.3",
33
"extraOrigins": [],
44
"sandbox": true,
55
"ssoSubIds": [],
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
timestamp=1730937600
2+
event=Hashtag Test
3+
subject=Warning [UNRAID] - #1 OS is cooking
4+
description=Disk 1 temperature has reached #epic # levels of proportion
5+
importance=warning
6+
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
timestamp=1730937600
2+
event=Temperature Test
3+
subject=Warning [UNRAID] - High disk temperature detected: 45&#8201;&#176;C
4+
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.
5+
importance=warning
6+

api/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@
116116
"graphql-subscriptions": "3.0.0",
117117
"graphql-tag": "2.12.6",
118118
"graphql-ws": "6.0.6",
119+
"html-entities": "^2.6.0",
119120
"ini": "5.0.0",
120121
"ip": "2.0.1",
121122
"jose": "6.0.13",

api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts

Lines changed: 64 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ import {
1414
} from '@app/unraid-api/graph/resolvers/notifications/notifications.model.js';
1515
import { NotificationsService } from '@app/unraid-api/graph/resolvers/notifications/notifications.service.js';
1616

17+
// Mock fs/promises for unit tests
18+
vi.mock('fs/promises', async () => {
19+
const actual = await vi.importActual<typeof import('fs/promises')>('fs/promises');
20+
const mockReadFile = vi.fn();
21+
return {
22+
...actual,
23+
readFile: mockReadFile,
24+
};
25+
});
26+
1727
// Mock getters.dynamix, Logger, and pubsub
1828
vi.mock('@app/store/index.js', () => {
1929
// Create test directory path inside factory function
@@ -61,24 +71,24 @@ const testNotificationsDir = join(tmpdir(), 'unraid-api-test-notifications');
6171

6272
describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
6373
let service: NotificationsService;
74+
let mockReadFile: any;
6475

65-
beforeEach(() => {
76+
beforeEach(async () => {
77+
const fsPromises = await import('fs/promises');
78+
mockReadFile = fsPromises.readFile as any;
79+
vi.mocked(mockReadFile).mockClear();
6680
service = new NotificationsService();
6781
});
6882

6983
it('should load and validate a valid notification file', async () => {
70-
const mockNotificationIni: NotificationIni = {
71-
timestamp: '1609459200',
72-
event: 'Test Event',
73-
subject: 'Test Subject',
74-
description: 'Test Description',
75-
importance: 'alert',
76-
link: 'http://example.com',
77-
};
78-
79-
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
80-
mockNotificationIni
81-
);
84+
const mockFileContent = `timestamp=1609459200
85+
event=Test Event
86+
subject=Test Subject
87+
description=Test Description
88+
importance=alert
89+
link=http://example.com`;
90+
91+
vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
8292

8393
const result = await (service as any).loadNotificationFile(
8494
'/test/path/test.notify',
@@ -99,17 +109,12 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
99109
});
100110

101111
it('should return masked warning notification on validation error (missing required fields)', async () => {
102-
const invalidNotificationIni: Omit<NotificationIni, 'event'> = {
103-
timestamp: '1609459200',
104-
// event: 'Missing Event', // missing required field
105-
subject: 'Test Subject',
106-
description: 'Test Description',
107-
importance: 'alert',
108-
};
109-
110-
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
111-
invalidNotificationIni
112-
);
112+
const mockFileContent = `timestamp=1609459200
113+
subject=Test Subject
114+
description=Test Description
115+
importance=alert`;
116+
117+
vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
113118

114119
const result = await (service as any).loadNotificationFile(
115120
'/test/path/invalid.notify',
@@ -121,17 +126,13 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
121126
});
122127

123128
it('should handle invalid enum values', async () => {
124-
const invalidNotificationIni: NotificationIni = {
125-
timestamp: '1609459200',
126-
event: 'Test Event',
127-
subject: 'Test Subject',
128-
description: 'Test Description',
129-
importance: 'not-a-valid-enum' as any,
130-
};
131-
132-
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
133-
invalidNotificationIni
134-
);
129+
const mockFileContent = `timestamp=1609459200
130+
event=Test Event
131+
subject=Test Subject
132+
description=Test Description
133+
importance=not-a-valid-enum`;
134+
135+
vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
135136

136137
const result = await (service as any).loadNotificationFile(
137138
'/test/path/invalid-enum.notify',
@@ -145,16 +146,12 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
145146
});
146147

147148
it('should handle missing description field (should return masked warning notification)', async () => {
148-
const mockNotificationIni: Omit<NotificationIni, 'description'> = {
149-
timestamp: '1609459200',
150-
event: 'Test Event',
151-
subject: 'Test Subject',
152-
importance: 'normal',
153-
};
154-
155-
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
156-
mockNotificationIni
157-
);
149+
const mockFileContent = `timestamp=1609459200
150+
event=Test Event
151+
subject=Test Subject
152+
importance=normal`;
153+
154+
vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
158155

159156
const result = await (service as any).loadNotificationFile(
160157
'/test/path/test.notify',
@@ -166,19 +163,15 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
166163
});
167164

168165
it('should preserve passthrough data from notification file (only known fields)', async () => {
169-
const mockNotificationIni: NotificationIni & { customField: string } = {
170-
timestamp: '1609459200',
171-
event: 'Test Event',
172-
subject: 'Test Subject',
173-
description: 'Test Description',
174-
importance: 'normal',
175-
link: 'http://example.com',
176-
customField: 'custom value',
177-
};
178-
179-
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
180-
mockNotificationIni
181-
);
166+
const mockFileContent = `timestamp=1609459200
167+
event=Test Event
168+
subject=Test Subject
169+
description=Test Description
170+
importance=normal
171+
link=http://example.com
172+
customField=custom value`;
173+
174+
vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
182175

183176
const result = await (service as any).loadNotificationFile(
184177
'/test/path/test.notify',
@@ -201,17 +194,12 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
201194
});
202195

203196
it('should handle missing timestamp field gracefully', async () => {
204-
const mockNotificationIni: Omit<NotificationIni, 'timestamp'> = {
205-
// timestamp is missing
206-
event: 'Test Event',
207-
subject: 'Test Subject',
208-
description: 'Test Description',
209-
importance: 'alert',
210-
};
211-
212-
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
213-
mockNotificationIni
214-
);
197+
const mockFileContent = `event=Test Event
198+
subject=Test Subject
199+
description=Test Description
200+
importance=alert`;
201+
202+
vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
215203

216204
const result = await (service as any).loadNotificationFile(
217205
'/test/path/missing-timestamp.notify',
@@ -225,17 +213,13 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
225213
});
226214

227215
it('should handle malformed timestamp field gracefully', async () => {
228-
const mockNotificationIni: NotificationIni = {
229-
timestamp: 'not-a-timestamp',
230-
event: 'Test Event',
231-
subject: 'Test Subject',
232-
description: 'Test Description',
233-
importance: 'alert',
234-
};
235-
236-
vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
237-
mockNotificationIni
238-
);
216+
const mockFileContent = `timestamp=not-a-timestamp
217+
event=Test Event
218+
subject=Test Subject
219+
description=Test Description
220+
importance=alert`;
221+
222+
vi.mocked(mockReadFile).mockResolvedValue(mockFileContent);
239223

240224
const result = await (service as any).loadNotificationFile(
241225
'/test/path/malformed-timestamp.notify',

api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { Injectable, Logger } from '@nestjs/common';
2-
import { readdir, rename, stat, unlink, writeFile } from 'fs/promises';
2+
import { readdir, readFile, rename, stat, unlink, writeFile } from 'fs/promises';
33
import { basename, join } from 'path';
44

55
import type { Stats } from 'fs';
66
import { FSWatcher, watch } from 'chokidar';
77
import { ValidationError } from 'class-validator';
88
import { execa } from 'execa';
99
import { emptyDir } from 'fs-extra';
10+
import { decode } from 'html-entities';
1011
import { encode as encodeIni } from 'ini';
1112
import { v7 as uuidv7 } from 'uuid';
1213

@@ -648,8 +649,11 @@ export class NotificationsService {
648649
* @throws File system errors (file not found, permission issues) or unexpected validation errors.
649650
*/
650651
private async loadNotificationFile(path: string, type: NotificationType): Promise<Notification> {
652+
const rawContent = await readFile(path, 'utf-8');
653+
const decodedContent = decode(rawContent);
654+
651655
const notificationFile = parseConfig<NotificationIni>({
652-
filePath: path,
656+
file: decodedContent,
653657
type: 'ini',
654658
});
655659

pnpm-lock.yaml

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)