Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Add spam checking for workflow bodies #18822

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a5fb61f
Add akismet package to tasker
joeauyeung Jan 23, 2025
8a63afa
Create scanWorkflowBody task
joeauyeung Jan 23, 2025
2db52be
Schedule workflow body scan
joeauyeung Jan 23, 2025
da358a4
Add AKISMET_API_KEY .env
joeauyeung Jan 23, 2025
38ca4fc
Auto lock user if spam is detected
joeauyeung Jan 23, 2025
60c5cdb
Uncommit key
joeauyeung Jan 23, 2025
5e447a1
Add safe param to workflow step
joeauyeung Jan 23, 2025
7067778
Migration for safe field
joeauyeung Jan 23, 2025
abd1b50
Do not process workflow steps is `safe` is false
joeauyeung Jan 23, 2025
01d2d58
Update migration to set previous records to true
joeauyeung Jan 23, 2025
834cb6b
Address comments
joeauyeung Jan 23, 2025
06e49cc
Refactor `scheduleWorkflowNotifications` to accept an object
joeauyeung Jan 24, 2025
7be1ec9
If new steps or editing old ones send to tasker
joeauyeung Jan 24, 2025
62d3726
Call `scheduleWorkflowNotifications` in task
joeauyeung Jan 24, 2025
0700a53
Fix `IS_SELF_HOSTED`
joeauyeung Jan 24, 2025
b88cd3e
Remove unused function
joeauyeung Jan 24, 2025
d096470
Make `safe` optional in schema
joeauyeung Jan 24, 2025
d9a0313
Type fix
joeauyeung Jan 24, 2025
c314420
Revert "Make `safe` optional in schema"
joeauyeung Jan 24, 2025
60ca133
Revert "Type fix"
joeauyeung Jan 24, 2025
2e569e3
Type fixes
joeauyeung Jan 24, 2025
bdfa6f9
Type fixes
joeauyeung Jan 24, 2025
a33e995
Address comments
joeauyeung Jan 24, 2025
2133ea3
Fix tests
joeauyeung Jan 24, 2025
60b0879
Add tests
joeauyeung Jan 24, 2025
5d2762f
Update tests
joeauyeung Jan 24, 2025
9f53fd0
Typo fix
joeauyeung Jan 24, 2025
49b3396
Update `safe` to `verifiedAt`
joeauyeung Jan 27, 2025
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
2 changes: 2 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,5 @@ DIRECTORY_IDS_TO_LOG=
# Set this when Cal.com is used to serve only one organization's booking pages
# Read more about it in the README.md
NEXT_PUBLIC_SINGLE_ORG_SLUG=

AKISMET_API_KEY="6451247bcfcd"
1 change: 1 addition & 0 deletions packages/features/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"@tanstack/react-table": "^8.9.3",
"@tanstack/react-virtual": "^3.10.9",
"@vercel/functions": "^1.4.0",
"akismet-api": "^6.0.0",
"framer-motion": "^10.12.8",
"lexical": "^0.9.0",
"react-select": "^5.7.0",
Expand Down
1 change: 1 addition & 0 deletions packages/features/tasker/tasker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type TaskPayloads = {
typeof import("./tasks/translateEventTypeData").ZTranslateEventDataPayloadSchema
>;
createCRMEvent: z.infer<typeof import("./tasks/crm/schema").createCRMEventSchema>;
scanWorkflowBody: z.infer<typeof import("./tasks/scanWorkflowBody").scanWorkflowBodySchema>;
};
export type TaskTypes = keyof TaskPayloads;
export type TaskHandler = (payload: string) => Promise<void>;
Expand Down
1 change: 1 addition & 0 deletions packages/features/tasker/tasks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const tasks: Record<TaskTypes, () => Promise<TaskHandler>> = {
translateEventTypeData: () =>
import("./translateEventTypeData").then((module) => module.translateEventTypeData),
createCRMEvent: () => import("./crm/createCRMEvent").then((module) => module.createCRMEvent),
scanWorkflowBody: () => import("./scanWorkflowBody").then((module) => module.scanWorkflowBody),
};

export default tasks;
53 changes: 53 additions & 0 deletions packages/features/tasker/tasks/scanWorkflowBody.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { AkismetClient } from "akismet-api";
import type { Comment } from "akismet-api";
import z from "zod";

import { lockUser } from "@calcom/lib/autoLock";
import { WEBAPP_URL } from "@calcom/lib/constants";
import logger from "@calcom/lib/logger";
import prisma from "@calcom/prisma";

export const scanWorkflowBodySchema = z.object({
userId: z.number(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need the userId to know which user to lock. This comes from the context of the workflow update tRPC endpoint.

workflowStepId: z.number(),
});

const log = logger.getSubLogger({ prefix: ["[tasker] scanWorkflowBody"] });

export async function scanWorkflowBody(payload: string) {
if (!process.env.AKISMET_API_KEY) {
log.warn("AKISMET_API_KEY not set, skipping scan");
joeauyeung marked this conversation as resolved.
Show resolved Hide resolved
return;
}

const { workflowStepId, userId } = scanWorkflowBodySchema.parse(JSON.parse(payload));

const workflowStep = await prisma.workflowStep.findUnique({
where: {
id: workflowStepId,
},
});

if (!workflowStep?.reminderBody) return;

const client = new AkismetClient({ key: process.env.AKISMET_API_KEY, blog: WEBAPP_URL });

const comment: Comment = {
user_ip: "127.0.0.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment needs an IP. From Akismet support we can just use this.

IP is used for scanning against know abuse IPs and adding a reputation score.

content: workflowStep.reminderBody,
};

const isSpam = await client.checkSpam(comment);

if (isSpam) {
log.warn(`Workflow step ${workflowStepId} is spam with body ${workflowStep.reminderBody}`);

await prisma.workflowStep.delete({
joeauyeung marked this conversation as resolved.
Show resolved Hide resolved
where: {
id: workflowStepId,
},
});

await lockUser("userId", userId.toString());
}
}
2 changes: 1 addition & 1 deletion packages/lib/autoLock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export async function handleAutoLock({
return false;
}

async function lockUser(identifierType: string, identifier: string) {
export async function lockUser(identifierType: string, identifier: string) {
if (!identifier) {
return;
}
Expand Down
18 changes: 18 additions & 0 deletions packages/trpc/server/routers/viewer/workflows/update.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
removeSmsReminderFieldForEventTypes,
isStepEdited,
getEmailTemplateText,
scheduleWorkflowBodyScan,
} from "./util";

type UpdateOptions = {
Expand Down Expand Up @@ -398,6 +399,13 @@ export const updateHandler = async ({ ctx, input }: UpdateOptions) => {
},
});

await scheduleWorkflowBodyScan({
Copy link
Member

Choose a reason for hiding this comment

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

In the update above, I think we need to set safe to false

Copy link
Member

Choose a reason for hiding this comment

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

But if we do that, then scheduling the updated reminders will fail too (scheduleWorkflowNotifications line https://github.com/calcom/cal.com/pull/18822/files#diff-e434eb7fdd98d2062613cf2c8f5a69bd969b932d5cd5b0ca329d434b118f6fa6R413

workflowStepId: oldStep.id,
userId: ctx.user.id,
newStepBody: newStep?.reminderBody,
oldStepBody: oldStep?.reminderBody,
});

// cancel all notifications of edited step
await WorkflowRepository.deleteAllWorkflowReminders(remindersFromStep);

Expand Down Expand Up @@ -466,6 +474,16 @@ export const updateHandler = async ({ ctx, input }: UpdateOptions) => {
)
);

await Promise.all(
createdSteps.map((step) =>
scheduleWorkflowBodyScan({
workflowStepId: step.id,
userId: ctx.user.id,
newStepBody: step.reminderBody,
})
)
);

// schedule notification for new step
await scheduleWorkflowNotifications(
Copy link
Member

Choose a reason for hiding this comment

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

same here, they would fail because tasker needs to run before it's set to true

activeOn,
Expand Down
26 changes: 25 additions & 1 deletion packages/trpc/server/routers/viewer/workflows/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
getSmsReminderNumberSource,
} from "@calcom/features/bookings/lib/getBookingFields";
import { removeBookingField, upsertBookingField } from "@calcom/features/eventtypes/lib/bookingFieldsManager";
import { SENDER_ID, SENDER_NAME } from "@calcom/lib/constants";
import tasker from "@calcom/features/tasker";
import { IS_SELF_HOSTED, SENDER_ID, SENDER_NAME } from "@calcom/lib/constants";
import { getBookerBaseUrl } from "@calcom/lib/getBookerUrl/server";
import getOrgIdFromMemberOrTeamId from "@calcom/lib/getOrgIdFromMemberOrTeamId";
import { getTeamIdFromEventType } from "@calcom/lib/getTeamIdFromEventType";
Expand Down Expand Up @@ -876,3 +877,26 @@ export function getEmailTemplateText(

return { emailBody, emailSubject };
}

export async function scheduleWorkflowBodyScan({
workflowStepId,
userId,
newStepBody,
oldStepBody,
}: {
workflowStepId: number;
userId: number;
newStepBody?: string | null;
oldStepBody?: string | null;
}) {
// Don't scan workflows for self hosters
if (IS_SELF_HOSTED || !process.env.AKISMET_API_KEY) return;

// If there is no body to scan then return
if (!newStepBody) return;

// If there is no change in body then return
if (newStepBody === oldStepBody) return;

await tasker.create("scanWorkflowBody", { workflowStepId, userId });
}
1 change: 1 addition & 0 deletions turbo.json
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@
"AB_TEST_BUCKET_PROBABILITY",
"ALLOWED_HOSTNAMES",
"ANALYZE",
"AKISMET_API_KEY",
"API_KEY_PREFIX",
"APP_ROUTER_APPS_CATEGORIES_CATEGORY_ENABLED",
"APP_ROUTER_APPS_CATEGORIES_ENABLED",
Expand Down
15 changes: 13 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4156,6 +4156,7 @@ __metadata:
"@testing-library/react-hooks": ^8.0.1
"@types/web-push": ^3.6.3
"@vercel/functions": ^1.4.0
akismet-api: ^6.0.0
framer-motion: ^10.12.8
lexical: ^0.9.0
react-select: ^5.7.0
Expand Down Expand Up @@ -18302,6 +18303,16 @@ __metadata:
languageName: node
linkType: hard

"akismet-api@npm:^6.0.0":
version: 6.0.0
resolution: "akismet-api@npm:6.0.0"
dependencies:
bluebird: ^3.1.1
superagent: ^8.0.0
checksum: f93a9eb11e83e63b8ab2217ee3122bac69a911d6b02d2f7158a8f1c4a8c262a6e52a7461a3b7afeab6930e601df95121aff2f7c90015ea69b0ac65090586e6e7
languageName: node
linkType: hard

"ansi-align@npm:^3.0.1":
version: 3.0.1
resolution: "ansi-align@npm:3.0.1"
Expand Down Expand Up @@ -19668,7 +19679,7 @@ __metadata:
languageName: node
linkType: hard

"bluebird@npm:^3.7.2":
"bluebird@npm:^3.1.1, bluebird@npm:^3.7.2":
version: 3.7.2
resolution: "bluebird@npm:3.7.2"
checksum: 869417503c722e7dc54ca46715f70e15f4d9c602a423a02c825570862d12935be59ed9c7ba34a9b31f186c017c23cac6b54e35446f8353059c101da73eac22ef
Expand Down Expand Up @@ -41437,7 +41448,7 @@ __metadata:
languageName: node
linkType: hard

"superagent@npm:^8.1.2":
"superagent@npm:^8.0.0, superagent@npm:^8.1.2":
version: 8.1.2
resolution: "superagent@npm:8.1.2"
dependencies:
Expand Down
Loading