Skip to content
Jump to bottom

POC of the auto unsubscribe functionality #1

Open
wants to merge 1 commit into
base: replay-376-base-e745a15
Choose a base branch
from
+42 −4
Open

POC of the auto unsubscribe functionality #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion apps/unsubscriber/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,13 @@ async function performFallbackUnsubscribe(page: Page): Promise<boolean> {
return false;
}

export async function autoUnsubscribe(url: string): Promise<boolean> {
export async function autoUnsubscribe({
url,
email,
}: {
url: string;
email: string;
}): Promise<boolean> {
Comment on lines +206 to +212
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's see what references to email exist in the file
rg -n "email" apps/unsubscriber/src/main.ts

Repository: admin-coderabbit/inbox-zero

Length of output: 220

🏁 Script executed:

#!/bin/bash
# Read the function implementation to see what it actually does
sed -n '206,276p' apps/unsubscriber/src/main.ts

Repository: admin-coderabbit/inbox-zero

Length of output: 2021

🏁 Script executed:

#!/bin/bash
# Check the signatures of helper functions to see if they should receive email
rg -A 3 "function (analyzePageWithAI|performUnsubscribeActions|performFallbackUnsubscribe)" apps/unsubscriber/src/main.ts | head -30

Repository: admin-coderabbit/inbox-zero

Length of output: 556

Critical: Unused parameter in function signature.

The email parameter is accepted in the function signature (line 208) but never used anywhere in the function body (lines 213-276). It's not passed to analyzePageWithAI, performUnsubscribeActions, performFallbackUnsubscribe, or any other helper function, and is never referenced in any form.

Either implement the email parameter by passing it to the appropriate helper functions (if needed for form filling or AI context), or remove it to avoid confusion and breaking changes.

🤖 Prompt for AI Agents 
In @apps/unsubscriber/src/main.ts around lines 206 - 212, The email parameter on
autoUnsubscribe is declared but never used; either remove it from
autoUnsubscribe's signature and all callers, or propagate it into the helper
calls that need user context: pass email into analyzePageWithAI,
performUnsubscribeActions, and performFallbackUnsubscribe (and update those
functions' signatures and internal logic to accept/use email for form-filling or
AI context). Ensure every call site and any types are updated consistently to
avoid breaking changes.

if (!isValidUrl(url)) {
console.error("Invalid URL provided:", url);
return false;
Expand Down
5 changes: 3 additions & 2 deletions apps/unsubscriber/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ if (env.CORS_ORIGIN) {

const unsubscribeSchema = z.object({
url: z.string().url(),
email: z.string().email(),
});

server.get("/", async (request, reply) => {
Expand All @@ -24,8 +25,8 @@ server.get("/", async (request, reply) => {

server.post("/unsubscribe", async (request, reply) => {
try {
const { url } = unsubscribeSchema.parse(request.body);
const success = await autoUnsubscribe(url);
const { url, email } = unsubscribeSchema.parse(request.body);
const success = await autoUnsubscribe({ url, email });
return {
success,
message: success
Expand Down
10 changes: 9 additions & 1 deletion apps/web/app/(app)/bulk-unsubscribe/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import React, { useCallback, useState } from "react";
import { toast } from "sonner";
import type { PostHog } from "posthog-js/react";
import { onAutoArchive, onDeleteFilter } from "@/utils/actions/client";
import { setNewsletterStatusAction } from "@/utils/actions/unsubscriber";
import {
setNewsletterStatusAction,
unsubscribeAction,
} from "@/utils/actions/unsubscriber";
import { decrementUnsubscribeCreditAction } from "@/utils/actions/premium";
import { NewsletterStatus } from "@prisma/client";
import { cleanUnsubscribeLink } from "@/utils/parse/parseHtml.client";
Expand Down Expand Up @@ -54,6 +57,10 @@ export function useUnsubscribe<T extends Row>({
posthog.capture("Clicked Unsubscribe");

if (item.status === NewsletterStatus.UNSUBSCRIBED) {
if (item.lastUnsubscribeLink) {
await unsubscribeAction({ url: item.lastUnsubscribeLink });
}
Comment on lines +60 to +62
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical logic error: unsubscribeAction called in the wrong flow.

Lines 59-68 handle the case where the newsletter is already unsubscribed (status === UNSUBSCRIBED). This is the "re-subscribe" or "toggle off" path that sets the status back to null. Calling unsubscribeAction here attempts to unsubscribe a user who is already unsubscribed, which is incorrect.

The unsubscribeAction should instead be called in the else block (line 70) where actual unsubscription happens.

🐛 Proposed fix 
      if (item.status === NewsletterStatus.UNSUBSCRIBED) {
-       if (item.lastUnsubscribeLink) {
-         await unsubscribeAction({ url: item.lastUnsubscribeLink });
-       }
-
        await setNewsletterStatusAction({
          newsletterEmail: item.name,
          status: null,
        });
        await mutate();
      } else {
+       if (item.lastUnsubscribeLink) {
+         await unsubscribeAction({ url: item.lastUnsubscribeLink });
+       }
        await unsubscribeAndArchive(item.name, mutate, refetchPremium);
      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (item.lastUnsubscribeLink) {
await unsubscribeAction({ url: item.lastUnsubscribeLink });
}
if (item.status === NewsletterStatus.UNSUBSCRIBED) {
await setNewsletterStatusAction({
newsletterEmail: item.name,
status: null,
});
await mutate();
} else {
if (item.lastUnsubscribeLink) {
await unsubscribeAction({ url: item.lastUnsubscribeLink });
}
await unsubscribeAndArchive(item.name, mutate, refetchPremium);
}
🤖 Prompt for AI Agents 
In @apps/web/app/(app)/bulk-unsubscribe/hooks.ts around lines 60 - 62, The code
is calling unsubscribeAction in the branch that handles status === UNSUBSCRIBED
(the re-subscribe/toggle-off flow), which is wrong; update the logic so that
when status === UNSUBSCRIBED you only set the item status back to null (or
perform the re-subscribe toggle) and do not call unsubscribeAction, and move the
await unsubscribeAction({ url: item.lastUnsubscribeLink }) call into the else
branch that handles the actual unsubscription path; reference the
status/UNSUBSCRIBED check, the item.lastUnsubscribeLink usage, and the
unsubscribeAction invocation in hooks.ts to make this change.

⚠️ Potential issue | 🟠 Major

Missing error handling for external API call.

The unsubscribeAction call has no specific error handling. If the external unsubscriber service fails, the user receives no feedback, and the status update proceeds regardless. This could lead to inconsistent state where the UI shows "unsubscribed" but the actual unsubscribe action failed.

Consider adding explicit error handling and user notification.

🛡️ Suggested improvement 
      if (item.lastUnsubscribeLink) {
-       await unsubscribeAction({ url: item.lastUnsubscribeLink });
+       const result = await unsubscribeAction({ url: item.lastUnsubscribeLink });
+       if (result.error) {
+         toast.error("Failed to unsubscribe. Please try again.");
+         setUnsubscribeLoading(false);
+         return;
+       }
      }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In @apps/web/app/(app)/bulk-unsubscribe/hooks.ts around lines 60 - 62, The code
calls unsubscribeAction({ url: item.lastUnsubscribeLink }) with no error
handling; wrap that call in a try/catch around unsubscribeAction (referencing
unsubscribeAction and item.lastUnsubscribeLink), on success proceed as before,
on failure log the error, surface a user-facing failure (e.g., return/throw an
error or set a failure status) and do not mark the item as unsubscribed; ensure
the caller/UI receives the error result so it can show a notification or retry
option.


await setNewsletterStatusAction({
newsletterEmail: item.name,
status: null,
Expand All @@ -72,6 +79,7 @@ export function useUnsubscribe<T extends Row>({
hasUnsubscribeAccess,
item.name,
item.status,
item.lastUnsubscribeLink,
mutate,
refetchPremium,
posthog,
Expand Down
2 changes: 2 additions & 0 deletions apps/web/env.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export const env = createEnv({
LICENSE_5_SEAT_VARIANT_ID: z.coerce.number().optional(),
LICENSE_10_SEAT_VARIANT_ID: z.coerce.number().optional(),
LICENSE_25_SEAT_VARIANT_ID: z.coerce.number().optional(),

UNSUBSCRIBER_API_URL: z.string().optional(),
},
client: {
NEXT_PUBLIC_LEMON_STORE_ID: z.string().nullish().default("inboxzero"),
Expand Down
21 changes: 21 additions & 0 deletions apps/web/utils/actions/unsubscriber.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { auth } from "@/app/api/auth/[...nextauth]/auth";
import prisma from "@/utils/prisma";
import type { NewsletterStatus } from "@prisma/client";
import { withActionInstrumentation } from "@/utils/actions/middleware";
import { env } from "@/env";

export const setNewsletterStatusAction = withActionInstrumentation(
"setNewsletterStatus",
Expand All @@ -30,3 +31,23 @@ export const setNewsletterStatusAction = withActionInstrumentation(
});
},
);

export const unsubscribeAction = withActionInstrumentation(
"unsubscribe",
async (options: { url: string }) => {
const session = await auth();
if (!session?.user.email) return { error: "Not logged in" };

const { url } = options;

const response = await fetch(
`${env.UNSUBSCRIBER_API_URL}/unsubscribe?url=${url}&email=${session.user.email}`,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Major: Sensitive data exposure in URL.

The user's email address is included in the URL query string. GET requests are logged in server logs, proxy logs, and browser history, exposing PII unnecessarily. Use a POST request with the email in the request body instead.

🤖 Prompt for AI Agents 
In @apps/web/utils/actions/unsubscriber.ts at line 44, The current fetch call
embeds session.user.email in the query string
(`${env.UNSUBSCRIBER_API_URL}/unsubscribe?url=${url}&email=${session.user.email}`),
exposing PII; change the request to POST to
`${env.UNSUBSCRIBER_API_URL}/unsubscribe` and move both url and
session.user.email into the JSON request body (e.g., { url, email:
session.user.email }), set the Content-Type: application/json header, and use
method: 'POST' so email is not leaked in logs or browser history; update the
corresponding call site in unsubscriber.ts (the fetch that builds that template
string) accordingly.

);
Comment on lines +43 to +45
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: HTTP method mismatch and missing URL encoding.

The client makes a GET request with query parameters, but the server endpoint at /unsubscribe is defined as POST (see apps/unsubscriber/src/server.ts line 26). This will result in a 404 or method not allowed error.

Additionally, the url and email parameters are not URL-encoded, creating potential injection vulnerabilities and breaking when URLs contain special characters.

🔧 Proposed fix 
-   const response = await fetch(
-     `${env.UNSUBSCRIBER_API_URL}/unsubscribe?url=${url}&email=${session.user.email}`,
-   );
+   const response = await fetch(
+     `${env.UNSUBSCRIBER_API_URL}/unsubscribe`,
+     {
+       method: 'POST',
+       headers: { 'Content-Type': 'application/json' },
+       body: JSON.stringify({ url, email: session.user.email }),
+     }
+   );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const response = await fetch(
`${env.UNSUBSCRIBER_API_URL}/unsubscribe?url=${url}&email=${session.user.email}`,
);
const response = await fetch(
`${env.UNSUBSCRIBER_API_URL}/unsubscribe`,
{
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ url, email: session.user.email }),
}
);
🤖 Prompt for AI Agents 
In @apps/web/utils/actions/unsubscriber.ts around lines 43 - 45, The fetch
currently issues a GET with raw query params which mismatches the server's POST
/unsubscribe and doesn't URL-encode inputs; change the fetch in unsubscriber.ts
to send a POST to `${env.UNSUBSCRIBER_API_URL}/unsubscribe` with a JSON body
containing url and session.user.email (or the expected body shape), use
encodeURIComponent if you must place values in a URL, and set the appropriate
header ('Content-Type': 'application/json'); ensure you JSON.stringify({ url,
email: session.user.email }) and handle the response as before.

⚠️ Potential issue | 🟠 Major

Major: Missing validation for optional environment variable.

env.UNSUBSCRIBER_API_URL is defined as optional in the environment configuration, but there's no check before using it. If it's not configured, the fetch will fail with a confusing error.

🛡️ Proposed fix 
    const { url } = options;

+   if (!env.UNSUBSCRIBER_API_URL) {
+     return { error: "Unsubscriber service not configured" };
+   }
+
    const response = await fetch(
      `${env.UNSUBSCRIBER_API_URL}/unsubscribe`,

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In @apps/web/utils/actions/unsubscriber.ts around lines 43 - 45,
env.UNSUBSCRIBER_API_URL is optional but used directly in the fetch call; add a
guard that checks env.UNSUBSCRIBER_API_URL before building the request (e.g.,
const base = env.UNSUBSCRIBER_API_URL) and if missing throw or return a clear
error message so the caller gets a helpful failure instead of an unclear runtime
error; then use that base when calling fetch and ensure you encode the query
params (url and session.user.email) when constructing the request URL.


if (!response.ok) {
return { error: "Failed to unsubscribe" };
}

return { success: true };
},
);