Holding-Only Checks Reapply a Bundle Discount

The short version

A bundle discount that applies automatically once several targets are held together must not be decided purely on the current holding state, “do they hold every target”. On that alone, once the set is complete, every further purchase of a slot outside the set keeps satisfying the holding condition, so a discount that should be one-time is recalculated and applied each time. The fix was to add one history check to the condition: “has this set’s discount ever been granted before”.

What it looks like

The system has a “bundle discount” that applies automatically to a company holding every target slot within one area. For a set covering three slots, applying while holding all three added the discount.

Whether the discount applied was decided by this logic.

// src/lib/promo-bundle.ts (the pre-fix decision logic)
let best: { discountYen: number; bundleId: string; bundleName: string } | null = null;
for (const b of active) {
  if (b.discount_yen <= 0) continue;
  const members = membersByBundle.get(b.id) ?? [];
  if (!members.includes(currentLoopId)) continue; // just in case
  // Do they already hold the qualifier slot (if any)?
  if (b.qualifier_loop_id && !heldLoops.has(b.qualifier_loop_id)) continue;
  // Do they already hold every member other than the current slot (= this is the last one)?
  const others = members.filter((m) => m !== currentLoopId);
  if (others.length === 0) continue;
  if (!others.every((m) => heldLoops.has(m))) continue;

  if (!best || b.discount_yen > best.discountYen)
    best = { discountYen: b.discount_yen, bundleId: b.id, bundleName: b.name };
}

heldLoops is the set of slots the company actively holds, fetched fresh each time. This decision reads as “is the slot being applied for right now the last one completing the set”, but what it actually checks is only “are they applying for some slot while holding every target”. Buy another slot outside the set after the set is complete and others.every((m) => heldLoops.has(m)) is still true, so the same discount is calculated again.

There is no cap on the number of slots in an area, so simply repeating purchases after completing a set applied a supposedly one-time discount on every purchase. Because it runs entirely through the legitimate application flow, this was exploitable as a money leak through self-service application.

Why

What this logic looked at was only the present snapshot — “what does this company hold right now” — and never the history of “has this company ever been granted this set’s discount”.

The inventory/capacity check (do they hold the target slots) and the discount’s idempotency check (have they already used it) are independent axes. The former is fine to re-evaluate on every application; the latter has a different property, “once it holds, never again”. This code implemented only the former, leaving it defenceless against a state that keeps satisfying the former — further purchases after set completion — which occurs perfectly normally.

Fixing it

We separately fetch whether the company has any contract (excluding cancelled) that was granted that set’s discount, and exclude already-granted sets from the candidates.

// src/lib/promo-bundle.ts (the added idempotency check)
// ⚠ Idempotency (money-leak prevention): a pack discount is once per company, once only.
//   Exclude any pack that already has a contract (other than cancelled) carrying its discount.
//   Without this, every further target slot bought after the pack is complete re-applies
//   the discount (exploitable via self-service application).
const { data: grantedRows } = await admin
  .from("contracts")
  .select("promo_bundle_id")
  .eq("company_id", companyId)
  .not("promo_bundle_id", "is", null)
  .neq("status", "cancelled");
const alreadyGranted = new Set(
  ((grantedRows ?? []) as { promo_bundle_id: string | null }[])
    .map((r) => r.promo_bundle_id)
    .filter((x): x is string => !!x)
);

Just before considering a set as a candidate, one line skips it if it is in alreadyGranted.

for (const b of active) {
  if (b.discount_yen <= 0) continue;
  if (alreadyGranted.has(b.id)) continue; // idempotent: this pack's discount is already granted
  const members = membersByBundle.get(b.id) ?? [];
  // ...(the holding checks unchanged)
}

The “do they hold every target” decision stays as it was, and adding one decision in front of it — “has this set’s discount already been granted” — makes the inventory check and the idempotency check work independently. The tests gained a case asserting the discount is null when one more target slot is added from a state where every target is held and a contract with that set’s discount already exists.

Why it wasn’t noticed

The main concern when this logic was first written was “produce the discount correctly at the moment the set is completed”. For that scenario the holding check on heldLoops alone works correctly. What was overlooked is a business rule outside the discount logic: after the set is complete, slots outside it can still be bought without limit. There is no bug in the holding check’s implementation; what actually happened is closer to not noticing that the condition can be satisfied repeatedly outside the assumed scenario.