From 0b3261c31e9d7e875cf1569f573d8a8ca98ce92f Mon Sep 17 00:00:00 2001 From: NubsCarson Date: Thu, 2 Jul 2026 19:08:13 +0000 Subject: [PATCH 1/2] fix(advertising): enforce ad-account approval + campaign status gate (#11364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ad spend is money movement, but any connected ad account was immediately active and campaigns never checked account status — a stolen/abusive account could spend with zero review, and a suspended account's campaigns could still be created/started (the declared pending/suspended states were never used). - connectAccount now creates accounts as 'pending' (was hardcoded 'active'). - approveAccount (pending->active) / rejectAccount (->suspended) — operator transitions, admin-gated at the route (requireAdmin), the same operator-executes posture as fiat payouts: an org owner can never self-approve their own ad account. - POST /api/v1/advertising/accounts/:id/approve|reject (requireAdmin). - createCampaign + startCampaign now refuse any account whose status !== active. Tests: 12/12 pass (bun test ad-account-approval.test.ts) — approval state machine (transitions, idempotency, non-pending guard, unknown account) and campaign spend blocked for pending/suspended/disconnected. Money-path — flagged for maintainer review; NOT self-merged. [cloud-security] --- .../api/v1/advertising/accounts/[id]/route.ts | 46 +++++- .../__tests__/ad-account-approval.test.ts | 152 ++++++++++++++++++ .../src/lib/services/advertising/index.ts | 69 +++++++- 3 files changed, 263 insertions(+), 4 deletions(-) create mode 100644 packages/cloud/shared/src/lib/services/__tests__/ad-account-approval.test.ts diff --git a/packages/cloud/api/v1/advertising/accounts/[id]/route.ts b/packages/cloud/api/v1/advertising/accounts/[id]/route.ts index 61d571bc2ebda..66ba56f22b3d4 100644 --- a/packages/cloud/api/v1/advertising/accounts/[id]/route.ts +++ b/packages/cloud/api/v1/advertising/accounts/[id]/route.ts @@ -1,11 +1,13 @@ /** - * 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 +55,42 @@ 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; diff --git a/packages/cloud/shared/src/lib/services/__tests__/ad-account-approval.test.ts b/packages/cloud/shared/src/lib/services/__tests__/ad-account-approval.test.ts new file mode 100644 index 0000000000000..d018dce830b68 --- /dev/null +++ b/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 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 = {}) { + 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/, + ); + }); + } +}); diff --git a/packages/cloud/shared/src/lib/services/advertising/index.ts b/packages/cloud/shared/src/lib/services/advertising/index.ts index d135721f9d704..7df5c05a1d784 100644 --- a/packages/cloud/shared/src/lib/services/advertising/index.ts +++ b/packages/cloud/shared/src/lib/services/advertising/index.ts @@ -223,7 +223,12 @@ class AdvertisingService { account_name: input.accountName || validation.accountName || "Ad Account", access_token_secret_id: accessTokenSecret.id, refresh_token_secret_id: refreshTokenSecretId, - status: "active", + // Ad spend is money movement, so a newly-connected account starts + // "pending" and cannot run campaigns until a platform operator approves + // it (POST /api/v1/advertising/accounts/:id/approve, requireAdmin) — the + // same operator-executes posture as fiat payouts/redemptions. This + // prevents a stolen/abusive ad account from spending before review. (#11364) + status: "pending", }); logger.info("[Advertising] Ad account connected", { @@ -234,6 +239,53 @@ class AdvertisingService { return account; } + /** + * Approve a pending ad account so it can run campaigns. Platform-operator + * action (requireAdmin at the route) — the same operator-executes posture as + * fiat payouts; an org owner can never self-approve their own ad account. (#11364) + */ + async approveAccount(accountId: string): Promise { + const account = await adAccountsRepository.findById(accountId); + if (!account) { + throw new Error("Ad account not found"); + } + if (account.status === "active") { + return account; // idempotent + } + if (account.status !== "pending") { + throw new Error( + `Ad account cannot be approved from status "${account.status}" (only "pending" accounts can be approved)`, + ); + } + const updated = await adAccountsRepository.updateStatus(accountId, "active"); + if (!updated) { + throw new Error("Ad account not found"); + } + logger.info("[Advertising] Ad account approved", { accountId }); + return updated; + } + + /** + * Reject or suspend an ad account so it cannot run campaigns. Platform-operator + * action (requireAdmin at the route). Covers both rejecting a pending account + * on review and suspending an active account for ToS. (#11364) + */ + async rejectAccount(accountId: string): Promise { + const account = await adAccountsRepository.findById(accountId); + if (!account) { + throw new Error("Ad account not found"); + } + if (account.status === "suspended") { + return account; // idempotent + } + const updated = await adAccountsRepository.updateStatus(accountId, "suspended"); + if (!updated) { + throw new Error("Ad account not found"); + } + logger.info("[Advertising] Ad account rejected/suspended", { accountId }); + return updated; + } + async disconnectAccount(accountId: string, organizationId: string): Promise { const account = await adAccountsRepository.findById(accountId); @@ -450,6 +502,13 @@ class AdvertisingService { if (!account || account.organization_id !== input.organizationId) { throw new Error("Ad account not found"); } + // Only an approved (active) ad account may spend — a pending/suspended/ + // disconnected account cannot create campaigns. (#11364) + if (account.status !== "active") { + throw new Error( + `Ad account is not active (status: ${account.status}); it must be approved before running campaigns`, + ); + } await contentSafetyService.assertSafeForPublicUse({ surface: "advertising_campaign", @@ -743,6 +802,14 @@ class AdvertisingService { if (!account) { throw new Error("Ad account not found"); } + // Only an approved (active) ad account may spend — block starting a campaign + // on a pending/suspended/disconnected account (e.g. suspended for ToS after + // the campaign was created). (#11364) + if (account.status !== "active") { + throw new Error( + `Ad account is not active (status: ${account.status}); it must be approved before running campaigns`, + ); + } const credentials = await this.getCredentials(account); const provider = this.getProvider(account.platform); From a7f90572e7a3cccd18bedeeb876213ceedf3ec56 Mon Sep 17 00:00:00 2001 From: Shaw Date: Thu, 2 Jul 2026 17:25:25 -0400 Subject: [PATCH 2/2] test(cloud): prove ad account approval admin gate --- ...advertising-account-approval-route.test.ts | 90 +++++++++++++++++++ .../api/v1/advertising/accounts/[id]/route.ts | 9 +- 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 packages/cloud/api/__tests__/advertising-account-approval-route.test.ts diff --git a/packages/cloud/api/__tests__/advertising-account-approval-route.test.ts b/packages/cloud/api/__tests__/advertising-account-approval-route.test.ts new file mode 100644 index 0000000000000..f99ab8cfbb996 --- /dev/null +++ b/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(); + }); +}); diff --git a/packages/cloud/api/v1/advertising/accounts/[id]/route.ts b/packages/cloud/api/v1/advertising/accounts/[id]/route.ts index 66ba56f22b3d4..e4f813eac3ce6 100644 --- a/packages/cloud/api/v1/advertising/accounts/[id]/route.ts +++ b/packages/cloud/api/v1/advertising/accounts/[id]/route.ts @@ -7,7 +7,10 @@ import { Hono } from "hono"; import { failureResponse } from "@/lib/api/cloud-worker-errors"; -import { requireAdmin, 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"; @@ -85,7 +88,9 @@ app.post("/reject", async (c) => { const account = await advertisingService.rejectAccount(id); - logger.info("[Advertising API] Account rejected/suspended", { accountId: id }); + logger.info("[Advertising API] Account rejected/suspended", { + accountId: id, + }); return c.json({ id: account.id, status: account.status }); } catch (error) {