From 9713d92250f433e40eb0532833e429f2af653f16 Mon Sep 17 00:00:00 2001 From: Peter Jeschke Date: Sun, 18 Jan 2026 20:06:25 +0100 Subject: [PATCH] Fix error on unknown notification type When a client queries the notification GET endpoint with an unknown type, it would get a 500 response due to a database error: > cause: PostgresError: invalid input value for enum notification_type: "reaction" (Example with Moshidon) Now, "unknown" notification types are filtered before handing them to the next layer. --- src/api/v1/notifications.test.ts | 31 ++++++++ src/api/v1/notifications.ts | 31 ++++---- src/api/v2/notifications.test.ts | 124 +++++++++++++++++++++++++++++++ src/api/v2/notifications.ts | 10 +++ 4 files changed, 181 insertions(+), 15 deletions(-) create mode 100644 src/api/v2/notifications.test.ts diff --git a/src/api/v1/notifications.test.ts b/src/api/v1/notifications.test.ts index 617a251e..6936ff75 100644 --- a/src/api/v1/notifications.test.ts +++ b/src/api/v1/notifications.test.ts @@ -333,4 +333,35 @@ describe.sequential("/api/v1/notifications", () => { }); }); }); + + describe("Notification types", () => { + it("can handle unknown notification types", async () => { + expect.assertions(2); + const accessToken = await getAccessToken(client, account, [ + "read:notifications", + ]); + + await createNotification( + account.id as Uuid, + "follow", + remoteAccount.id, + new Date(), + ); + + const response = await app.request( + "/api/v1/notifications?types[]=SurelyInvalidType", + { + method: "GET", + headers: { + authorization: bearerAuthorization(accessToken), + }, + }, + ); + + expect(response.status).toBe(200); + + const body = await response.json(); + expect(body).toHaveLength(0); + }); + }); }); diff --git a/src/api/v1/notifications.ts b/src/api/v1/notifications.ts index cc1897cc..d403bdf6 100644 --- a/src/api/v1/notifications.ts +++ b/src/api/v1/notifications.ts @@ -13,7 +13,14 @@ import { tokenRequired, type Variables, } from "../../oauth/middleware"; -import { notifications, polls, pollVotes, posts } from "../../schema"; +import { + type NotificationType, + notifications, + notificationTypeEnum, + polls, + pollVotes, + posts, +} from "../../schema"; import type { Uuid } from "../../uuid"; const logger = getLogger(["hollo", "notifications"]); @@ -59,20 +66,11 @@ function parseNotificationId(compositeId: string): ParsedNotificationId { const app = new Hono<{ Variables: Variables }>(); -export type NotificationType = - | "mention" - | "status" - | "reblog" - | "follow" - | "follow_request" - | "favourite" - | "emoji_reaction" - | "poll" - | "update" - | "admin.sign_up" - | "admin.report" - | "quote" - | "quoted_update"; +// set for O(1) access to all possible types +const notificationTypeSet = new Set(notificationTypeEnum.enumValues); +function isNotificationType(value: string) { + return notificationTypeSet.has(value as NotificationType); +} app.get( "/", @@ -121,6 +119,9 @@ app.get( "quoted_update", ]; } + // types contains client-supplied values, which are not necessarily valid NotificationType. Filter everything we don't know and prevent problems later + // excludeTypes doesn't need filtering because we won't pass it along + types = types.filter(isNotificationType); types = types.filter((t) => !excludeTypes?.includes(t)); const startTime = performance.now(); diff --git a/src/api/v2/notifications.test.ts b/src/api/v2/notifications.test.ts new file mode 100644 index 00000000..ddc4f77a --- /dev/null +++ b/src/api/v2/notifications.test.ts @@ -0,0 +1,124 @@ +import { beforeEach, describe, expect, it } from "vitest"; + +import { cleanDatabase } from "../../../tests/helpers"; +import { + bearerAuthorization, + createAccount, + createOAuthApplication, + getAccessToken, +} from "../../../tests/helpers/oauth"; + +import db from "../../db"; +import app from "../../index"; +import * as Schema from "../../schema"; +import type { Uuid } from "../../uuid"; + +// Helper to create a remote account for use as notification actor +async function createRemoteAccount(username: string): Promise { + const accountId = crypto.randomUUID() as Uuid; + const accountIri = `https://remote.test/@${username}`; + + await db + .insert(Schema.instances) + .values({ + host: "remote.test", + software: "mastodon", + softwareVersion: null, + }) + .onConflictDoNothing(); + + const [account] = await db + .insert(Schema.accounts) + .values({ + id: accountId, + iri: accountIri, + instanceHost: "remote.test", + type: "Person", + name: `Remote: ${username}`, + emojis: {}, + handle: `@${username}@remote.test`, + bioHtml: "", + url: accountIri, + protected: false, + inboxUrl: `${accountIri}/inbox`, + followersUrl: `${accountIri}/followers`, + sharedInboxUrl: "https://remote.test/inbox", + featuredUrl: `${accountIri}/pinned`, + published: new Date(), + }) + .returning(); + + return account; +} + +// Helper to create a notification +async function createNotification( + accountOwnerId: Uuid, + type: Schema.NotificationType, + actorAccountId: Uuid, + createdAt?: Date, +): Promise { + const id = crypto.randomUUID() as Uuid; + const created = createdAt ?? new Date(); + + const [notification] = await db + .insert(Schema.notifications) + .values({ + id, + accountOwnerId, + type, + actorAccountId, + groupKey: `ungrouped-${id}`, + created, + }) + .returning(); + + return notification; +} + +describe.sequential("/api/v2/notifications", () => { + let client: Awaited>; + let account: Awaited>; + let remoteAccount: Schema.Account; + + beforeEach(async () => { + await cleanDatabase(); + + account = await createAccount(); + remoteAccount = await createRemoteAccount("remote_user"); + client = await createOAuthApplication({ + scopes: ["read:notifications"], + }); + }); + + describe("Notification types", () => { + it("can handle unknown notification types", async () => { + expect.assertions(2); + const accessToken = await getAccessToken(client, account, [ + "read:notifications", + ]); + + await createNotification( + account.id as Uuid, + "follow", + remoteAccount.id, + new Date(), + ); + + const response = await app.request( + "/api/v2/notifications?types[]=SurelyInvalidType", + { + method: "GET", + headers: { + authorization: bearerAuthorization(accessToken), + }, + }, + ); + + expect(response.status).toBe(200); + + const body = await response.json(); + expect(body.notification_groups).toHaveLength(0); + }); + }); +}); diff --git a/src/api/v2/notifications.ts b/src/api/v2/notifications.ts index 4cdb8aec..e277a6fd 100644 --- a/src/api/v2/notifications.ts +++ b/src/api/v2/notifications.ts @@ -17,6 +17,7 @@ import { type NotificationType, notificationGroups, notifications, + notificationTypeEnum, posts, } from "../../schema"; import type { Uuid } from "../../uuid"; @@ -31,6 +32,12 @@ function formatNotificationId(created: Date, type: string, id: string): string { return `${created.toISOString()}/${type}/${id}`; } +// set for O(1) access to all possible types +const notificationTypeSet = new Set(notificationTypeEnum.enumValues); +function isNotificationType(value: string) { + return notificationTypeSet.has(value as NotificationType); +} + // GET /api/v2/notifications - Get grouped notifications app.get( "/", @@ -76,6 +83,9 @@ app.get( "quoted_update", ]; } + // types contains client-supplied values, which are not necessarily valid NotificationType. Filter everything we don't know and prevent problems later + // excludeTypes doesn't need filtering because we won't pass it along + types = types.filter(isNotificationType); types = types.filter((t) => !excludeTypes?.includes(t)); const startTime = performance.now();