Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions server/src/controllers/notificationController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,28 @@ import { AppError } from "@/utils/AppError.js";
import { INotificationsService } from "@/service/index.js";
import { requireTeamId, requireUserId } from "./controllerUtils.js";
import { IMonitorsRepository } from "@/repositories/index.js";
import type { Notification } from "@/types/notification.js";

const SENSITIVE_FIELDS: (keyof Notification)[] = ["accessToken", "accountSid"];

const MASK_SUFFIX = "****";

const maskValue = (value: string): string => {
if (value.length <= 4) return MASK_SUFFIX;
return value.slice(0, 4) + MASK_SUFFIX;
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but it's a pretty fragile solution and an undocumented contract 🤔

Why don't we return a sentinel value like how setting the Pagespeed API key works? I think that's a more robust solution. It requires a bit of fiddling around on the back and front end but makes for better UX


const isMasked = (value: string): boolean => value.endsWith(MASK_SUFFIX);

const maskSensitiveFields = (notification: Notification): Notification => {
const masked = { ...notification };
for (const field of SENSITIVE_FIELDS) {
if (masked[field]) {
(masked as Record<string, unknown>)[field] = maskValue(masked[field] as string);
}
}
return masked;
};
Comment on lines +20 to +30

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is untested as far as I can tell, lets add some tests for this to keep coverage up


const SERVICE_NAME = "NotificationController";

Expand Down Expand Up @@ -62,7 +84,7 @@ class NotificationController implements INotificationController {
return res.status(200).json({
success: true,
msg: "Notification created successfully",
data: notification,
data: maskSensitiveFields(notification),
});
} catch (error) {
next(error);
Expand All @@ -77,7 +99,7 @@ class NotificationController implements INotificationController {
return res.status(200).json({
success: true,
msg: "Notifications fetched successfully",
data: notifications,
data: notifications.map(maskSensitiveFields),
});
} catch (error) {
next(error);
Expand Down Expand Up @@ -109,7 +131,7 @@ class NotificationController implements INotificationController {
return res.status(200).json({
success: true,
msg: "Notification fetched successfully",
data: notification,
data: maskSensitiveFields(notification),
});
} catch (error) {
next(error);
Expand All @@ -124,11 +146,19 @@ class NotificationController implements INotificationController {
const teamId = requireTeamId(req.user?.teamId);
const notificationId = validatedParams.id;

const editedNotification = await this.notificationsService.updateById(notificationId, teamId, validatedBody);
// Strip masked sensitive fields so they don't overwrite real values
const updateData = { ...validatedBody };
for (const field of SENSITIVE_FIELDS) {
if (updateData[field as keyof typeof updateData] && isMasked(updateData[field as keyof typeof updateData] as string)) {
delete updateData[field as keyof typeof updateData];
}
}

const editedNotification = await this.notificationsService.updateById(notificationId, teamId, updateData);
return res.status(200).json({
success: true,
msg: "Notification updated successfully",
data: editedNotification,
data: maskSensitiveFields(editedNotification),
});
} catch (error) {
next(error);
Expand Down
Loading