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

[NubsCarson]

Problem (#11364)

Ad spend is money movement, but the approval workflow + status field were entirely unenforced:

• connectAccount hardcoded status: "active" — every connected account was immediately usable, no review.
• The declared pending/suspended states were never set anywhere.
• createCampaign and startCampaign never checked account.status — so a stolen/abusive account could spend with zero review, and even a suspended account's campaigns could be created/started.

Fix

Mirrors the codebase's existing money-safety posture (fiat payouts/redemptions are requireAdmin operator-executed steps — a user can never self-trigger money movement):

1. connectAccount now creates accounts pending.
2. approveAccount (pending→active) / rejectAccount (→suspended) service transitions — idempotent, with a guard that only pending can be approved.
3. POST /api/v1/advertising/accounts/:id/approve | /reject — both requireAdmin, so an org owner can never self-approve their own ad account (platform operator executes).
4. createCampaign + startCampaign now refuse any account whose status !== "active".

Evidence

bun test ad-account-approval.test.ts → 12/12 pass, 0 fail:

• approve: pending→active persisted · idempotent on active (no write) · refuses non-pending · throws on unknown
• reject: active→suspended persisted · idempotent on suspended
• createCampaign blocked when account is pending/suspended/disconnected (×3)
• startCampaign blocked when account is pending/suspended/disconnected (×3)

Tests the real advertisingService; only the repository boundary is spied (no mock.module).

Behavior-change note for reviewers

New ad accounts now require a platform operator to approve before running campaigns (default pending). This is the intended anti-abuse posture per #11364, consistent with the admin-gated payout flow — but it does introduce an operator step. If you'd prefer to keep active-by-default and only enforce the suspend path, that's a one-line change to connectAccount; flagging the policy choice for your call.

Money-path → please review + merge; not self-merged. [cloud-security]

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9728609c-b83d-4d52-8c4c-bf6d15a4bba9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/11364-ad-account-approval

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[NubsCarson]

[cloud-security] review — the PR correctly does what it claims (pending-by-default connect, admin-only approve/reject failing closed via requireAdmin, create/start gated before any credit charge; no conflict with the merged #11369 refund logic or open #11497). Two adjacent money gaps worth a fast-follow (not blocking this merge):

1. [med, money] updateCampaign isn't gated on account.status (advertising/index.ts:~701) — a suspended/rejected account can still raise live ad spend via a budget increase. Add the same if (account.status !== "active") throw guard you use on create/start (at minimum when input.budgetAmount increases spend, ideally for any live-pushing change), before the platform push + credit deduct.
2. [med] rejectAccount only flips DB status — it doesn't pause already-running campaigns (:~273), which keep spending on the external platform after the account is rejected. Enumerate the account's active campaigns and best-effort provider.pauseCampaign + updateStatus('paused') each (log failures), or explicitly document reject as approval-gate-only.

I'll ship these two as a fast-follow PR referencing this one. Money-path merge → @lalalune per charter.

[NubsCarson]

[cloud-audit] COMMENT — approval gate verified correct; suspension enforcement is incomplete (non-blocking follow-up).

Invariant checked: a never-approved account can never spend or serve — HOLDS on every path I traced.

• createCampaign gates account.status !== "active" at packages/cloud/shared/src/lib/services/advertising/index.ts:507 before the first creditsService.deductCredits (:522) — fail-closed, no charge, no refund path needed.
• startCampaign gates at :808 before provider.activateCampaign (:817).
• Creatives transitively require a campaign of the same org (:1050-1052), and a campaign can only exist if the account was active at creation — so pending accounts cannot reach createCreative's deduct either.
• requireAdmin (packages/cloud/shared/src/lib/auth/workers-hono-auth.ts:306-336) resolves platform admin via adminService.getAdminStatusForUser and fails closed on lookup error — it is not an org-role check, so an org owner genuinely cannot self-approve. ✓
• "Looks right but won't apply" class: clean. status is a plain text column and the AdAccountStatus union already declared pending/suspended — no enum/CHECK, no migration needed. The route file is already mounted (app.route("/api/v1/advertising/accounts/:id", …) in _router.generated.ts), and Hono .route() flattens sub-paths, so app.post("/approve") serves POST …/accounts/:id/approve with :id accessible (same param mechanics as the existing x402/requests/[id]/settle route). No competing mount shadows it.
• Tests exercise the real service with repository-boundary spies only; the 12 cases cover the state machine + both enforcement points including the disconnected status. ✓

Caveats (suspension direction — the approval gate is fine, but rejectAccount's claimed ToS-suspension coverage is partial):

1. rejectAccount doesn't stop a running campaign (index.ts:273-287): it only flips account status. The account's already-active campaigns keep serving and spending — both on the external platform (never paused) and in the internal SSP, whose eligibility query findEligibleAd (packages/cloud/shared/src/db/repositories/ad-slots.ts:110-134) filters only campaign.status/creative.status with no join to ad_accounts.status.
2. updateCampaign has no status gate (index.ts:670-717): a suspended account can still push a live budget increase (deduct + provider.updateCampaign goes live). Not reachable from pending accounts, so not an approval bypass — but it lets a ToS-suspended advertiser escalate live spend.
3. createCreative (index.ts:1049-1070) deducts credits with no account gate — same suspended-only reachability.

None of these lets an unapproved account spend, so I'm not blocking on them — but since the PR body claims reject "covers suspending an active account for ToS", please either add the status gate to updateCampaign/createCreative + account-status filter to findEligibleAd (and ideally pause active campaigns in rejectAccount) in this PR, or file a follow-up issue so the gap is tracked. Minor style note: sub-actions elsewhere use directories ([id]/start/route.ts); [id]/approve/route.ts would match convention, though the in-file registration does mount correctly.

Policy note for the merger: this makes new ad-account connection operator-gated (pending by default) — intended per #11364, flagged by the author, and consistent with the payout posture. Existing accounts are grandfathered active (no backfill), which is the sane default.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[lalalune]

Reviewed after rebasing onto current origin/develop and pushed an additional API-level route test proving the admin gate.\n\nLocal verification after final push:\n- bun test src/lib/services/tests/ad-account-approval.test.ts (packages/cloud/shared): 12 pass / 0 fail\n- bun test tests/advertising-account-approval-route.test.ts (packages/cloud/api): 3 pass / 0 fail\n- bunx @biomejs/biome@2.5.2 check packages/cloud/api/v1/advertising/accounts/[id]/route.ts packages/cloud/api/tests/advertising-account-approval-route.test.ts packages/cloud/shared/src/lib/services/advertising/index.ts packages/cloud/shared/src/lib/services/tests/ad-account-approval.test.ts\n- bun run --cwd packages/cloud/shared typecheck && bun run --cwd packages/cloud/api typecheck\n- git diff --check origin/develop...HEAD\n\nBehavior reviewed: newly connected ad accounts start pending; only admin routes can approve/reject; create/start campaign reject pending/suspended/disconnected accounts before spend.

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.