company_id Survives Contract End, Owner Check Isn't Enough

A placement’s company_id doesn’t clear when the contract ends. An auto-approval that only checks “is this our placement” lets a former client post to an ended slot with no review at all.

The short version

The company_id column on an ad placement never clears when a contract ends and the slot’s status reverts to “ended” or “open” again. The first version of the auto-approval feature decided whether to skip human review by checking only “does the submitting company’s company_id match the placement’s company_id” — which let a former client auto-approve creatives onto a slot whose contract had already ended. The gap was flagged in code review the same day the feature shipped (Reviewer B-1), and closed before it ever reached production by adding a function, isAutoApprovable, that checks the slot’s live status and the contract’s validity on every approval.

What it looks like

• placements.company_id is a foreign key set once, at booking time, to the company that reserved the slot — and it is never cleared when the contract ends or the slot is resold
• The first implementation of auto-approval used only one safety check: does the submitting company’s company_id match the placement’s company_id
• That match proves the company once booked the slot — not that it currently holds it
• After the contract ends, while the slot’s availability sits at 空き (“open”), 商談中 (“in talks”), or 終了 (“ended”), the former client’s account still sees it as “our placement” and can submit creatives against it
• With the automation policy (auto_approve_creatives) turned on, that submission skips human review and goes straight to delivery

Why

The commit that first introduced auto-approval documented its safety reasoning like this:

// creative-auto-approve.ts (b1138b8c40, lines 16-35)
/**
 * Auto-approval for submitted creatives (auto_approve_creatives policy / monetization plan §6).
 *
 * Preconditions (safety basis):
 *  - Only submissions that already **passed** server-side spec re-validation call this
 *    (inside submitCreative — the client is untrusted).
 *  - A customer account only exists once the contract is "signed" (Flow A). So being able to
 *    log in and submit at all already means the ad terms (including content rules) are agreed to.
 *  - The placement was atomically booked to this company at application time — it can't be
 *    overwritten by another company (submitCreative already verified the company_id match).
 */
export async function autoApproveCreative(opts: {
  creativeId: string;
  placementId: string;
  companyId: string;
  title: string;
}): Promise<{ approved: boolean }> {

The third point — “the placement was atomically booked to this company at application time” — is true at the moment of booking. But whether that booking is still valid today is a different question, and the column definition doesn’t answer it:

-- 0023_self_serve_booking.sql (6f62043a46, lines 24-27)
add column if not exists availability      text not null default '空き'
  check (availability in ('空き','商談中','申込済','掲載中','終了')),
add column if not exists company_id        uuid references public.companies(id) on delete set null,
add column if not exists contract_id       uuid references public.contracts(id) on delete set null,

(availability’s five values translate to: 空き “open”, 商談中 “in talks”, 申込済 “booked”, 掲載中 “live”, 終了 “ended”.)

company_id’s on delete set null only fires if the company row itself is deleted. When a contract ends and availability flips to 終了 (“ended”), company_id stays exactly as it was. So the check the first implementation relied on — “does company_id match” — only proves the placement was booked by this company at some point; it says nothing about whether the contract is still active now.

Fixing it

The fix adds a function that queries the live state of the slot and its contract on every single approval, instead of trusting the stale foreign key.

// creative-auto-approve.ts (98e80a37ef, lines 16-53)
/** 自動承認できる枠の販売状態。申込済=契約手続き完了（署名時に遷移）/ 掲載中=配信中素材の差し替え。 */
const AUTO_APPROVABLE_AVAILABILITY = ["申込済", "掲載中"];

/**
 * 自動承認の前提検証（Reviewer B-1）: 枠の販売状態と契約の有効性を**毎回その場で**確認する。
 * placements.company_id は契約終了後も残留するため（0023）、「自社の枠」チェックだけでは
 * 元顧客が終了枠/再販売中の空き枠に無審査で掲載できてしまう。以下を全て満たすときだけ自動承認:
 *  - 枠が自社・販売状態が 申込済/掲載中（空き・商談中・終了は不可）
 *  - 枠に契約が紐付いており、その契約が active・署名済・掲載期間内
 * 満たさない場合は自動承認せず「審査中」（人間の審査キュー）に残す。
 */
async function isAutoApprovable(
  admin: ReturnType<typeof createSupabaseAdminClient>,
  placementId: string,
  companyId: string
): Promise<boolean> {
  const { data: slot } = await admin
    .from("placements")
    .select("id, company_id, availability, contract_id")
    .eq("id", placementId)
    .maybeSingle();
  if (!slot || slot.company_id !== companyId) return false;
  if (!AUTO_APPROVABLE_AVAILABILITY.includes(slot.availability as string)) return false;
  if (!slot.contract_id) return false;

  const { data: ct } = await admin
    .from("contracts")
    .select("status, application_status, period_end")
    .eq("id", slot.contract_id)
    .maybeSingle();
  if (!ct) return false;
  const today = new Date().toISOString().slice(0, 10);
  return (
    ct.status === "active" &&
    ct.application_status === "署名済" &&
    (!ct.period_end || (ct.period_end as string) >= today)
  );
}

(The Japanese identifiers above are runtime string values, left as-is: 申込済 = “booked”/application complete, 掲載中 = “live”, 署名済 = “signed”.)

This keeps the original company_id match in place, but adds a check for the slot’s current state on top of it: is availability 申込済 (“booked”) or 掲載中 (“live”), and is the linked contract active, 署名済 (“signed”), and within its run dates. The caller doesn’t proceed to the approval RPC at all unless both hold.

// creative-auto-approve.ts (98e80a37ef, lines 55-86)
export async function autoApproveCreative(opts: {
  creativeId: string;
  placementId: string;
  companyId: string;
  title: string;
  mediaType: "image" | "video" | null;
}): Promise<{ approved: boolean }> {
  const { creativeId, placementId, companyId, title } = opts;
  // B-2: images only. Video can't have its runtime enforced server-side, so skip auto-approval.
  if (opts.mediaType !== "image") return { approved: false };

  const admin = createSupabaseAdminClient();

  // B-1: verify slot status and contract validity (fall through to manual review if not satisfied)
  if (!(await isAutoApprovable(admin, placementId, companyId))) {
    return { approved: false };
  }

The call site was also changed to be explicit about which media type it’s passing:

// (portal)/creatives/actions.ts (98e80a37ef, lines 120-132)
// Automation policy: auto-approve and move to Flow B (immediate delivery) if spec check
// passed and a placement is linked. Images only (video duration can't be verified
// server-side). Slot status and contract validity are checked inside autoApproveCreative;
// if not satisfied, it stays "pending review" in the manual queue (fail-closed).
if (validPlacementId) {
  const auto = await getAutomationSettings();
  if (auto.auto_approve_creatives) {
    const r = await autoApproveCreative({
      creativeId: inserted.id as string,
      placementId: validPlacementId,
      companyId: user.companyId,
      title,
      mediaType: media,
    });

Whenever isAutoApprovable returns false, the item lands in the same fail-closed path as an RPC failure: it stays “pending review” in the human queue. The existing approval logic (the approve_creative_and_enqueue RPC itself) was left untouched.

Preventing a repeat

This gap was caught in code review the same day the automation feature shipped — the commit log’s “fix per reviewer” wording, and the fix commit landing immediately after the introducing commit, both show that. The source files have no record of it ever being exploited in production.

Structurally, the gap existed in the first, pre-review implementation because two separate questions — “do we own this” and “is the current contract still valid” — were collapsed into a single check. A foreign key persists as a record of “this relationship once existed,” but whether that relationship is still valid has to come from another table — here, contracts.status / application_status / period_end. When an ownership check and a validity check happen to look like the same condition, it’s easy to check one and assume you’ve covered both. Whenever two tables are joined by a foreign key but the relationship between them can expire, “are they linked” and “is that link still active” need to be written as two explicit, separate checks.