From 136f126296af65f50da598d084d1485c0e40437a Mon Sep 17 00:00:00 2001 From: Mohamed Bassem Date: Sun, 27 Apr 2025 00:02:20 +0100 Subject: feat: Implement generic rule engine (#1318) * Add schema for the new rule engine * Add rule engine backend logic * Implement the worker logic and event firing * Implement the UI changesfor the rule engine * Ensure that when a referenced list or tag are deleted, the corresponding event/action is * Dont show smart lists in rule engine events * Add privacy validations for attached tag and list ids * Move the rules logic into a models --- packages/trpc/routers/_app.ts | 2 + packages/trpc/routers/bookmarks.ts | 27 +++ packages/trpc/routers/lists.ts | 3 +- packages/trpc/routers/rules.test.ts | 379 ++++++++++++++++++++++++++++++++++++ packages/trpc/routers/rules.ts | 120 ++++++++++++ packages/trpc/routers/tags.ts | 2 +- 6 files changed, 531 insertions(+), 2 deletions(-) create mode 100644 packages/trpc/routers/rules.test.ts create mode 100644 packages/trpc/routers/rules.ts (limited to 'packages/trpc/routers') diff --git a/packages/trpc/routers/_app.ts b/packages/trpc/routers/_app.ts index 7af19884..394e95e7 100644 --- a/packages/trpc/routers/_app.ts +++ b/packages/trpc/routers/_app.ts @@ -7,6 +7,7 @@ import { feedsAppRouter } from "./feeds"; import { highlightsAppRouter } from "./highlights"; import { listsAppRouter } from "./lists"; import { promptsAppRouter } from "./prompts"; +import { rulesAppRouter } from "./rules"; import { tagsAppRouter } from "./tags"; import { usersAppRouter } from "./users"; import { webhooksAppRouter } from "./webhooks"; @@ -23,6 +24,7 @@ export const appRouter = router({ highlights: highlightsAppRouter, webhooks: webhooksAppRouter, assets: assetsAppRouter, + rules: rulesAppRouter, }); // export type definition of API export type AppRouter = typeof appRouter; diff --git a/packages/trpc/routers/bookmarks.ts b/packages/trpc/routers/bookmarks.ts index 9a1b6b0b..b9a21400 100644 --- a/packages/trpc/routers/bookmarks.ts +++ b/packages/trpc/routers/bookmarks.ts @@ -45,6 +45,7 @@ import { AssetPreprocessingQueue, LinkCrawlerQueue, OpenAIQueue, + triggerRuleEngineOnEvent, triggerSearchDeletion, triggerSearchReindex, triggerWebhook, @@ -430,6 +431,11 @@ export const bookmarksAppRouter = router({ break; } } + await triggerRuleEngineOnEvent(bookmark.id, [ + { + type: "bookmarkAdded", + }, + ]); await triggerSearchReindex(bookmark.id); await triggerWebhook(bookmark.id, "created"); return bookmark; @@ -573,6 +579,17 @@ export const bookmarksAppRouter = router({ /* includeContent: */ false, ); + if (input.favourited === true || input.archived === true) { + await triggerRuleEngineOnEvent( + input.bookmarkId, + [ + ...(input.favourited === true ? ["favourited" as const] : []), + ...(input.archived === true ? ["archived" as const] : []), + ].map((t) => ({ + type: t, + })), + ); + } // Trigger re-indexing and webhooks await triggerSearchReindex(input.bookmarkId); await triggerWebhook(input.bookmarkId, "edited"); @@ -1141,6 +1158,16 @@ export const bookmarksAppRouter = router({ ), ); + await triggerRuleEngineOnEvent(input.bookmarkId, [ + ...idsToRemove.map((t) => ({ + type: "tagRemoved" as const, + tagId: t, + })), + ...allIds.map((t) => ({ + type: "tagAdded" as const, + tagId: t, + })), + ]); await triggerSearchReindex(input.bookmarkId); await triggerWebhook(input.bookmarkId, "edited"); return { diff --git a/packages/trpc/routers/lists.ts b/packages/trpc/routers/lists.ts index 12960316..65cffd2d 100644 --- a/packages/trpc/routers/lists.ts +++ b/packages/trpc/routers/lists.ts @@ -38,7 +38,8 @@ export const listsAppRouter = router({ .output(zBookmarkListSchema) .use(ensureListOwnership) .mutation(async ({ input, ctx }) => { - return await ctx.list.update(input); + await ctx.list.update(input); + return ctx.list.list; }), merge: authedProcedure .input(zMergeListSchema) diff --git a/packages/trpc/routers/rules.test.ts b/packages/trpc/routers/rules.test.ts new file mode 100644 index 00000000..6bbbcd84 --- /dev/null +++ b/packages/trpc/routers/rules.test.ts @@ -0,0 +1,379 @@ +import { beforeEach, describe, expect, test } from "vitest"; + +import { RuleEngineRule } from "@karakeep/shared/types/rules"; + +import type { CustomTestContext } from "../testUtils"; +import { defaultBeforeEach } from "../testUtils"; + +describe("Rules Routes", () => { + let tagId1: string; + let tagId2: string; + let otherUserTagId: string; + + let listId: string; + let otherUserListId: string; + + beforeEach(async (ctx) => { + await defaultBeforeEach(true)(ctx); + + tagId1 = ( + await ctx.apiCallers[0].tags.create({ + name: "Tag 1", + }) + ).id; + + tagId2 = ( + await ctx.apiCallers[0].tags.create({ + name: "Tag 2", + }) + ).id; + + otherUserTagId = ( + await ctx.apiCallers[1].tags.create({ + name: "Tag 1", + }) + ).id; + + listId = ( + await ctx.apiCallers[0].lists.create({ + name: "List 1", + icon: "😘", + }) + ).id; + + otherUserListId = ( + await ctx.apiCallers[1].lists.create({ + name: "List 1", + icon: "😘", + }) + ).id; + }); + + test("create rule with valid data", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; + + const validRuleInput: Omit = { + name: "Valid Rule", + description: "A test rule", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [ + { type: "addTag", tagId: tagId1 }, + { type: "addToList", listId: listId }, + ], + }; + + const createdRule = await api.create(validRuleInput); + expect(createdRule).toMatchObject({ + name: "Valid Rule", + description: "A test rule", + enabled: true, + event: validRuleInput.event, + condition: validRuleInput.condition, + actions: validRuleInput.actions, + }); + }); + + test("create rule fails with invalid data (no actions)", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; + + const invalidRuleInput: Omit = { + name: "Invalid Rule", + description: "Missing actions", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [], // Empty actions array - should fail validation + }; + + await expect(() => api.create(invalidRuleInput)).rejects.toThrow( + /You must specify at least one action/, + ); + }); + + test("create rule fails with invalid event (empty tagId)", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; + + const invalidRuleInput: Omit = { + name: "Invalid Rule", + description: "Invalid event", + enabled: true, + event: { type: "tagAdded", tagId: "" }, // Empty tagId - should fail + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId1 }], + }; + + await expect(() => api.create(invalidRuleInput)).rejects.toThrow( + /You must specify a tag for this event type/, + ); + }); + + test("create rule fails with invalid condition (empty tagId in hasTag)", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; + + const invalidRuleInput: Omit = { + name: "Invalid Rule", + description: "Invalid condition", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "hasTag", tagId: "" }, // Empty tagId - should fail + actions: [{ type: "addTag", tagId: tagId1 }], + }; + + await expect(() => api.create(invalidRuleInput)).rejects.toThrow( + /You must specify a tag for this condition type/, + ); + }); + + test("update rule with valid data", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; + + // First, create a rule + const createdRule = await api.create({ + name: "Original Rule", + description: "Original desc", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId1 }], + }); + + const validUpdateInput: RuleEngineRule = { + id: createdRule.id, + name: "Updated Rule", + description: "Updated desc", + enabled: false, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "removeTag", tagId: tagId2 }], + }; + + const updatedRule = await api.update(validUpdateInput); + expect(updatedRule).toMatchObject({ + id: createdRule.id, + name: "Updated Rule", + description: "Updated desc", + enabled: false, + event: validUpdateInput.event, + condition: validUpdateInput.condition, + actions: validUpdateInput.actions, + }); + }); + + test("update rule fails with invalid data (empty action tagId)", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; + + // First, create a rule + const createdRule = await api.create({ + name: "Original Rule", + description: "Original desc", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId1 }], + }); + + const invalidUpdateInput: RuleEngineRule = { + id: createdRule.id, + name: "Updated Rule", + description: "Updated desc", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "removeTag", tagId: "" }], // Empty tagId - should fail + }; + + await expect(() => api.update(invalidUpdateInput)).rejects.toThrow( + /You must specify a tag for this action type/, + ); + }); + + test("delete rule", async ({ apiCallers }) => { + const api = apiCallers[0].rules; + + const createdRule = await api.create({ + name: "Rule to Delete", + description: "", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId1 }], + }); + + await api.delete({ id: createdRule.id }); + + // Attempt to fetch the rule should fail + await expect(() => + api.update({ ...createdRule, name: "Updated" }), + ).rejects.toThrow(/Rule not found/); + }); + + test("list rules", async ({ apiCallers }) => { + const api = apiCallers[0].rules; + + await api.create({ + name: "Rule 1", + description: "", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId1 }], + }); + + await api.create({ + name: "Rule 2", + description: "", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId2 }], + }); + + const rulesList = await api.list(); + expect(rulesList.rules.length).toBeGreaterThanOrEqual(2); + expect(rulesList.rules.some((rule) => rule.name === "Rule 1")).toBeTruthy(); + expect(rulesList.rules.some((rule) => rule.name === "Rule 2")).toBeTruthy(); + }); + + describe("privacy checks", () => { + test("cannot access or manipulate another user's rule", async ({ + apiCallers, + }) => { + const apiUserA = apiCallers[0].rules; // First user + const apiUserB = apiCallers[1].rules; // Second user + + // User A creates a rule + const createdRule = await apiUserA.create({ + name: "User A's Rule", + description: "A rule for User A", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId1 }], + }); + + // User B tries to update User A's rule + const updateInput: RuleEngineRule = { + id: createdRule.id, + name: "Trying to Update", + description: "Unauthorized update", + enabled: true, + event: createdRule.event, + condition: createdRule.condition, + actions: createdRule.actions, + }; + + await expect(() => apiUserB.update(updateInput)).rejects.toThrow( + /Rule not found/, + ); + }); + + test("cannot create rule with event on another user's tag", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; // First user trying to use second user's tag + + const invalidRuleInput: Omit = { + name: "Invalid Rule", + description: "Event with other user's tag", + enabled: true, + event: { type: "tagAdded", tagId: otherUserTagId }, // Other user's tag + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId1 }], + }; + + await expect(() => api.create(invalidRuleInput)).rejects.toThrow( + /Tag not found/, // Expect an error indicating lack of ownership + ); + }); + + test("cannot create rule with condition on another user's tag", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; // First user trying to use second user's tag + + const invalidRuleInput: Omit = { + name: "Invalid Rule", + description: "Condition with other user's tag", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "hasTag", tagId: otherUserTagId }, // Other user's tag + actions: [{ type: "addTag", tagId: tagId1 }], + }; + + await expect(() => api.create(invalidRuleInput)).rejects.toThrow( + /Tag not found/, + ); + }); + + test("cannot create rule with action on another user's tag", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; // First user trying to use second user's tag + + const invalidRuleInput: Omit = { + name: "Invalid Rule", + description: "Action with other user's tag", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: otherUserTagId }], // Other user's tag + }; + + await expect(() => api.create(invalidRuleInput)).rejects.toThrow( + /Tag not found/, + ); + }); + + test("cannot create rule with event on another user's list", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; // First user trying to use second user's list + + const invalidRuleInput: Omit = { + name: "Invalid Rule", + description: "Event with other user's list", + enabled: true, + event: { type: "addedToList", listId: otherUserListId }, // Other user's list + condition: { type: "alwaysTrue" }, + actions: [{ type: "addTag", tagId: tagId1 }], + }; + + await expect(() => api.create(invalidRuleInput)).rejects.toThrow( + /List not found/, + ); + }); + + test("cannot create rule with action on another user's list", async ({ + apiCallers, + }) => { + const api = apiCallers[0].rules; // First user trying to use second user's list + + const invalidRuleInput: Omit = { + name: "Invalid Rule", + description: "Action with other user's list", + enabled: true, + event: { type: "bookmarkAdded" }, + condition: { type: "alwaysTrue" }, + actions: [{ type: "addToList", listId: otherUserListId }], // Other user's list + }; + + await expect(() => api.create(invalidRuleInput)).rejects.toThrow( + /List not found/, + ); + }); + }); +}); diff --git a/packages/trpc/routers/rules.ts b/packages/trpc/routers/rules.ts new file mode 100644 index 00000000..5def8003 --- /dev/null +++ b/packages/trpc/routers/rules.ts @@ -0,0 +1,120 @@ +import { experimental_trpcMiddleware, TRPCError } from "@trpc/server"; +import { and, eq, inArray } from "drizzle-orm"; +import { z } from "zod"; + +import { bookmarkTags } from "@karakeep/db/schema"; +import { + RuleEngineRule, + zNewRuleEngineRuleSchema, + zRuleEngineRuleSchema, + zUpdateRuleEngineRuleSchema, +} from "@karakeep/shared/types/rules"; + +import { AuthedContext, authedProcedure, router } from "../index"; +import { List } from "../models/lists"; +import { RuleEngineRuleModel } from "../models/rules"; + +const ensureRuleOwnership = experimental_trpcMiddleware<{ + ctx: AuthedContext; + input: { id: string }; +}>().create(async (opts) => { + const rule = await RuleEngineRuleModel.fromId(opts.ctx, opts.input.id); + return opts.next({ + ctx: { + ...opts.ctx, + rule, + }, + }); +}); + +const ensureTagListOwnership = experimental_trpcMiddleware<{ + ctx: AuthedContext; + input: Omit; +}>().create(async (opts) => { + const tagIds = [ + ...(opts.input.event.type === "tagAdded" || + opts.input.event.type === "tagRemoved" + ? [opts.input.event.tagId] + : []), + ...(opts.input.condition.type === "hasTag" + ? [opts.input.condition.tagId] + : []), + ...opts.input.actions.flatMap((a) => + a.type == "addTag" || a.type == "removeTag" ? [a.tagId] : [], + ), + ]; + + const validateTags = async () => { + if (tagIds.length == 0) { + return; + } + const userTags = await opts.ctx.db.query.bookmarkTags.findMany({ + where: and( + eq(bookmarkTags.userId, opts.ctx.user.id), + inArray(bookmarkTags.id, tagIds), + ), + columns: { + id: true, + }, + }); + if (tagIds.some((t) => userTags.find((u) => u.id == t) == null)) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "Tag not found", + }); + } + }; + + const listIds = [ + ...(opts.input.event.type === "addedToList" || + opts.input.event.type === "removedFromList" + ? [opts.input.event.listId] + : []), + ...opts.input.actions.flatMap((a) => + a.type == "addToList" || a.type == "removeFromList" ? [a.listId] : [], + ), + ]; + + const [_tags, _lists] = await Promise.all([ + validateTags(), + Promise.all(listIds.map((l) => List.fromId(opts.ctx, l))), + ]); + return opts.next(); +}); + +export const rulesAppRouter = router({ + create: authedProcedure + .input(zNewRuleEngineRuleSchema) + .output(zRuleEngineRuleSchema) + .use(ensureTagListOwnership) + .mutation(async ({ input, ctx }) => { + const newRule = await RuleEngineRuleModel.create(ctx, input); + return newRule.rule; + }), + update: authedProcedure + .input(zUpdateRuleEngineRuleSchema) + .output(zRuleEngineRuleSchema) + .use(ensureRuleOwnership) + .use(ensureTagListOwnership) + .mutation(async ({ ctx, input }) => { + await ctx.rule.update(input); + return ctx.rule.rule; + }), + delete: authedProcedure + .input(z.object({ id: z.string() })) + .use(ensureRuleOwnership) + .mutation(async ({ ctx }) => { + await ctx.rule.delete(); + }), + list: authedProcedure + .output( + z.object({ + rules: z.array(zRuleEngineRuleSchema), + }), + ) + .query(async ({ ctx }) => { + return { + rules: (await RuleEngineRuleModel.getAll(ctx)).map((r) => r.rule), + }; + }), +}); diff --git a/packages/trpc/routers/tags.ts b/packages/trpc/routers/tags.ts index cdf47f4f..7f75c16e 100644 --- a/packages/trpc/routers/tags.ts +++ b/packages/trpc/routers/tags.ts @@ -18,7 +18,7 @@ function conditionFromInput(input: { tagId: string }, userId: string) { return and(eq(bookmarkTags.id, input.tagId), eq(bookmarkTags.userId, userId)); } -const ensureTagOwnership = experimental_trpcMiddleware<{ +export const ensureTagOwnership = experimental_trpcMiddleware<{ ctx: Context; input: { tagId: string }; }>().create(async (opts) => { -- cgit v1.2.3-70-g09d2