fix(advertising): enforce ad-account approval + campaign status gate (#11364)

packages/cloud/api/__tests__/advertising-account-approval-route.test.ts

@@ -0,0 +1,90 @@
+/**
+ * Route-level guard for ad-account approval/rejection (#11364).
+ *
+ * The shared service owns the state machine, but the money-path invariant that
+ * an org owner cannot self-approve lives at the Hono route via `requireAdmin`.
+ * Drive the real route so a regression to `requireUserOrApiKeyWithOrg` is caught
+ * before any service transition can run.
+ */
+
+import { beforeEach, describe, expect, mock, test } from "bun:test";
+import { ForbiddenError } from "@elizaos/cloud-shared/lib/api/cloud-worker-errors";
+import { Hono } from "hono";
+import * as workersHonoAuthActual from "@/lib/auth/workers-hono-auth";
+
+const ACCOUNT_ID = "00000000-0000-4000-8000-0000000000ad";
+
+const requireAdmin = mock();
+mock.module("@/lib/auth/workers-hono-auth", () => ({
+  ...workersHonoAuthActual,
+  requireAdmin,
+}));
+
+const approveAccount = mock();
+const rejectAccount = mock();
+mock.module("@/lib/services/advertising", () => ({
+  advertisingService: {
+    approveAccount,
+    rejectAccount,
+  },
+}));
+
+mock.module("@/lib/utils/logger", () => ({
+  logger: { error: mock(), info: mock(), warn: mock(), debug: mock() },
+}));
+
+const { default: accountRoute } = await import(
+  "../v1/advertising/accounts/[id]/route"
+);
+
+const app = new Hono();
+app.route("/api/v1/advertising/accounts/:id", accountRoute);
+
+function post(path: "approve" | "reject") {
+  return app.request(`/api/v1/advertising/accounts/${ACCOUNT_ID}/${path}`, {
+    method: "POST",
+  });
+}
+
+beforeEach(() => {
+  requireAdmin.mockReset();
+  approveAccount.mockReset();
+  rejectAccount.mockReset();
+});
+
+describe("advertising account approve/reject routes", () => {
+  test("approve rejects a non-admin caller before touching the service", async () => {
+    requireAdmin.mockRejectedValue(ForbiddenError("Admin access required"));
+
+    const res = await post("approve");
+    const body = (await res.json()) as { code?: string; error?: string };
+
+    expect(res.status).toBe(403);
+    expect(body).toMatchObject({
+      code: "access_denied",
+      error: "Admin access required",
+    });
+    expect(approveAccount).not.toHaveBeenCalled();
+  });
+
+  test("approve lets an admin transition the account", async () => {
+    requireAdmin.mockResolvedValue({ user: { id: "admin-1" }, role: "admin" });
+    approveAccount.mockResolvedValue({ id: ACCOUNT_ID, status: "active" });
+
+    const res = await post("approve");
+    const body = (await res.json()) as { id?: string; status?: string };
+
+    expect(res.status).toBe(200);
+    expect(body).toEqual({ id: ACCOUNT_ID, status: "active" });
+    expect(approveAccount).toHaveBeenCalledWith(ACCOUNT_ID);
+  });
+
+  test("reject is also admin-gated before touching the service", async () => {
+    requireAdmin.mockRejectedValue(ForbiddenError("Admin access required"));
+
+    const res = await post("reject");
+
+    expect(res.status).toBe(403);
+    expect(rejectAccount).not.toHaveBeenCalled();
+  });
+});

packages/cloud/api/v1/advertising/accounts/[id]/route.ts

@@ -1,11 +1,16 @@
 /**
- * GET    /api/v1/advertising/accounts/[id] — get a specific ad account.
- * DELETE /api/v1/advertising/accounts/[id] — disconnect an ad account.
+ * GET    /api/v1/advertising/accounts/[id]         — get a specific ad account.
+ * DELETE /api/v1/advertising/accounts/[id]         — disconnect an ad account.
+ * POST   /api/v1/advertising/accounts/[id]/approve — approve a pending account (admin).
+ * POST   /api/v1/advertising/accounts/[id]/reject  — reject/suspend an account (admin).
  */
 
 import { Hono } from "hono";
 import { failureResponse } from "@/lib/api/cloud-worker-errors";
-import { requireUserOrApiKeyWithOrg } from "@/lib/auth/workers-hono-auth";
+import {
+  requireAdmin,
+  requireUserOrApiKeyWithOrg,
+} from "@/lib/auth/workers-hono-auth";
 import { advertisingService } from "@/lib/services/advertising";
 import { logger } from "@/lib/utils/logger";
 import type { AppEnv } from "@/types/cloud-worker-env";
@@ -53,4 +58,44 @@ app.delete("/", async (c) => {
   }
 });
 
+/**
+ * Approve a pending ad account (platform operator only). requireAdmin ensures an
+ * org owner can never self-approve their own account — the same operator-executes
+ * posture as fiat payouts. (#11364)
+ */
+app.post("/approve", async (c) => {
+  try {
+    await requireAdmin(c);
+    const id = c.req.param("id")!;
+
+    const account = await advertisingService.approveAccount(id);
+
+    logger.info("[Advertising API] Account approved", { accountId: id });
+
+    return c.json({ id: account.id, status: account.status });
+  } catch (error) {
+    return failureResponse(c, error);
+  }
+});
+
+/**
+ * Reject or suspend an ad account (platform operator only). (#11364)
+ */
+app.post("/reject", async (c) => {
+  try {
+    await requireAdmin(c);
+    const id = c.req.param("id")!;
+
+    const account = await advertisingService.rejectAccount(id);
+
+    logger.info("[Advertising API] Account rejected/suspended", {
+      accountId: id,
+    });
+
+    return c.json({ id: account.id, status: account.status });
+  } catch (error) {
+    return failureResponse(c, error);
+  }
+});
+
 export default app;

packages/cloud/shared/src/lib/services/__tests__/ad-account-approval.test.ts

@@ -0,0 +1,152 @@
+/**
+ * Ad-account approval gate + campaign status enforcement (#11364).
+ *
+ * Ad spend is money movement, so before this fix any connected ad account was
+ * immediately "active" and campaigns never checked account status — a stolen or
+ * abusive account could spend with zero review, and a suspended account's
+ * campaigns could still be created/started. This locks it down:
+ *
+ *  - connectAccount now creates accounts "pending" (asserted in the connect flow
+ *    tests; here we cover the state machine + enforcement directly).
+ *  - approveAccount (pending→active) / rejectAccount (→suspended) are the
+ *    platform-operator transitions (requireAdmin at the route — an org owner can
+ *    never self-approve, same posture as fiat payouts).
+ *  - createCampaign and startCampaign refuse any account whose status !== "active".
+ *
+ * Tests the REAL advertisingService; only the repository boundary is spied
+ * (no `mock.module`, so nothing leaks).
+ */
+
+import { afterEach, describe, expect, spyOn, test } from "bun:test";
+import { adAccountsRepository, adCampaignsRepository } from "../../../db/repositories";
+import type { AdAccount, AdAccountStatus } from "../../../db/schemas/ad-accounts";
+import { advertisingService } from "../advertising";
+
+const ORG_ID = "org-1";
+const ACCOUNT_ID = "acct-1";
+const CAMPAIGN_ID = "campaign-1";
+
+const spies: Array<{ mockRestore: () => void }> = [];
+function track<T extends { mockRestore: () => void }>(s: T): T {
+  spies.push(s);
+  return s;
+}
+
+afterEach(() => {
+  for (const s of spies.splice(0)) s.mockRestore();
+});
+
+function makeAccount(status: AdAccountStatus): AdAccount {
+  return {
+    id: ACCOUNT_ID,
+    organization_id: ORG_ID,
+    connected_by_user_id: "user-1",
+    platform: "meta",
+    external_account_id: "ext-1",
+    account_name: "Acct",
+    access_token_secret_id: "sec-1",
+    refresh_token_secret_id: null,
+    status,
+    metadata: {},
+    created_at: new Date(),
+    updated_at: new Date(),
+  } as unknown as AdAccount;
+}
+
+function makeCampaign(over: Record<string, unknown> = {}) {
+  return {
+    id: CAMPAIGN_ID,
+    organization_id: ORG_ID,
+    ad_account_id: ACCOUNT_ID,
+    name: "My Campaign",
+    external_campaign_id: "ext-camp-1",
+    ...over,
+  } as never;
+}
+
+describe("approveAccount (#11364)", () => {
+  test("pending → active, persisted via updateStatus", async () => {
+    track(spyOn(adAccountsRepository, "findById").mockResolvedValue(makeAccount("pending")));
+    const update = track(
+      spyOn(adAccountsRepository, "updateStatus").mockResolvedValue(makeAccount("active")),
+    );
+
+    const result = await advertisingService.approveAccount(ACCOUNT_ID);
+
+    expect(result.status).toBe("active");
+    expect(update).toHaveBeenCalledWith(ACCOUNT_ID, "active");
+  });
+
+  test("is idempotent on an already-active account (no write)", async () => {
+    track(spyOn(adAccountsRepository, "findById").mockResolvedValue(makeAccount("active")));
+    const update = track(spyOn(adAccountsRepository, "updateStatus").mockResolvedValue(undefined));
+
+    const result = await advertisingService.approveAccount(ACCOUNT_ID);
+
+    expect(result.status).toBe("active");
+    expect(update).not.toHaveBeenCalled();
+  });
+
+  test("refuses to approve from a non-pending status (e.g. suspended)", async () => {
+    track(spyOn(adAccountsRepository, "findById").mockResolvedValue(makeAccount("suspended")));
+    const update = track(spyOn(adAccountsRepository, "updateStatus").mockResolvedValue(undefined));
+
+    await expect(advertisingService.approveAccount(ACCOUNT_ID)).rejects.toThrow(/only "pending"/);
+    expect(update).not.toHaveBeenCalled();
+  });
+
+  test("throws on unknown account", async () => {
+    track(spyOn(adAccountsRepository, "findById").mockResolvedValue(undefined));
+    await expect(advertisingService.approveAccount(ACCOUNT_ID)).rejects.toThrow(/not found/);
+  });
+});
+
+describe("rejectAccount (#11364)", () => {
+  test("active → suspended, persisted", async () => {
+    track(spyOn(adAccountsRepository, "findById").mockResolvedValue(makeAccount("active")));
+    const update = track(
+      spyOn(adAccountsRepository, "updateStatus").mockResolvedValue(makeAccount("suspended")),
+    );
+
+    const result = await advertisingService.rejectAccount(ACCOUNT_ID);
+
+    expect(result.status).toBe("suspended");
+    expect(update).toHaveBeenCalledWith(ACCOUNT_ID, "suspended");
+  });
+
+  test("is idempotent on an already-suspended account (no write)", async () => {
+    track(spyOn(adAccountsRepository, "findById").mockResolvedValue(makeAccount("suspended")));
+    const update = track(spyOn(adAccountsRepository, "updateStatus").mockResolvedValue(undefined));
+
+    const result = await advertisingService.rejectAccount(ACCOUNT_ID);
+
+    expect(result.status).toBe("suspended");
+    expect(update).not.toHaveBeenCalled();
+  });
+});
+
+describe("campaign spend requires an approved (active) account (#11364)", () => {
+  for (const status of ["pending", "suspended", "disconnected"] as AdAccountStatus[]) {
+    test(`createCampaign is blocked when account is ${status}`, async () => {
+      track(spyOn(adAccountsRepository, "findById").mockResolvedValue(makeAccount(status)));
+
+      await expect(
+        advertisingService.createCampaign({
+          organizationId: ORG_ID,
+          adAccountId: ACCOUNT_ID,
+          name: "Campaign",
+          budgetAmount: 100,
+        } as never),
+      ).rejects.toThrow(/not active/);
+    });
+
+    test(`startCampaign is blocked when account is ${status}`, async () => {
+      track(spyOn(adCampaignsRepository, "findById").mockResolvedValue(makeCampaign()));
+      track(spyOn(adAccountsRepository, "findById").mockResolvedValue(makeAccount(status)));
+
+      await expect(advertisingService.startCampaign(CAMPAIGN_ID, ORG_ID)).rejects.toThrow(
+        /not active/,
+      );
+    });
+  }
+});