Harden Freebuff country gating

[jahooma]

Blocks Freebuff access for IPinfo hosting and service privacy signals, fails closed when IPinfo privacy lookup cannot complete, and prefers Cloudflare's client IP header for the privacy check. Persists country/privacy audit metadata on free_session, including resolved country, CF/GeoIP countries, privacy signals, block reason, HMAC client IP hash, and checked timestamp. Adds the Drizzle migration and updates Freebuff waiting room docs/tests. Validated with focused Freebuff session/country tests plus common, internal, and web typechecks.

[greptile-apps]

Greptile Summary

This PR hardens Freebuff country gating by adding IPinfo hosting/service blocking, fail-closed behavior when the privacy lookup fails, Cloudflare header preference for the IP used in the privacy check, and persistence of country/privacy audit metadata on free_session. The migration, schema, types, handlers, and tests all land together cleanly.

Confidence Score: 5/5

Safe to merge; all remaining findings are P2 style/simplification suggestions that do not affect correctness.

The core gating logic is correct and well-tested. The migration is additive and non-breaking. The fail-closed behavior is explicit and intentional. All open comments are P2 simplification opportunities per the custom instruction, none of which affect runtime correctness.

web/src/server/free-mode-country.ts and web/src/app/api/v1/freebuff/session/_handlers.ts have the most opportunity for simplification.

Important Files Changed

Filename	Overview
web/src/server/free-mode-country.ts	Core country-gate logic: IPinfo privacy lookup with in-process cache, fail-closed on lookup failure, HMAC IP hashing. Has a small simplification opportunity with the getIpPrivacy wrapper.
web/src/app/api/v1/freebuff/session/_handlers.ts	POST/GET/DELETE handlers: country gate runs on every GET poll (not just POST), which means a cache miss during IPinfo downtime blocks queued users mid-poll.
web/src/server/free-session/store.ts	Country metadata columns wired into UPSERT via countryAccessColumns helper; cleanly spreads into both INSERT values and ON CONFLICT SET.
packages/internal/src/db/migrations/0047_tough_silver_fox.sql	Non-breaking additive migration: seven nullable columns added to free_session; no data loss risk.
packages/internal/src/db/schema.ts	New nullable country/privacy columns added to freeSession table; types match migration SQL.
web/src/server/free-session/types.ts	Adds FreeSessionCountryAccessMetadata interface and optional country fields to InternalSessionRow; straightforward type additions.
web/src/server/tests/free-mode-country.test.ts	Good test coverage: CF header preference, hosting/service blocking, fail-closed on lookup failure, IP hash, IPinfo signal parsing.
common/src/types/freebuff-session.ts	Adds ip_privacy_lookup_failed to FreebuffCountryBlockReason discriminated union.

Comments Outside Diff (3)

1. web/src/server/free-mode-country.ts, line 185-202 (link)

   getIpPrivacy wrapper can be inlined

   This function only (a) selects between the injected stub and the real implementation, and (b) wraps the call in a try-catch. Both concerns can live directly in getFreeModeCountryAccess, removing an indirection layer that makes the control flow harder to follow. The function and its options coupling can then be removed entirely, keeping the privacy-lookup path in one place.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: web/src/server/free-mode-country.ts
   Line: 185-202
   
   Comment:
   **`getIpPrivacy` wrapper can be inlined**
   
   This function only (a) selects between the injected stub and the real implementation, and (b) wraps the call in a try-catch. Both concerns can live directly in `getFreeModeCountryAccess`, removing an indirection layer that makes the control flow harder to follow. The function and its `options` coupling can then be removed entirely, keeping the privacy-lookup path in one place.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

2. web/src/server/free-mode-country.ts, line 108-129 (link)

   Cache eviction iterates all entries on every write

   The setIpinfoPrivacyCache cleanup loop (lines 113–117) walks the entire map on every write to remove expired entries. With up to 5 000 entries this is O(n) per write. A simpler approach: skip the expired-eviction loop entirely and rely only on the size-cap eviction — expired entries are harmless since get re-checks expiresAt before returning. The size cap already bounds memory; stale entries will be evicted as new ones arrive.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: web/src/server/free-mode-country.ts
   Line: 108-129
   
   Comment:
   **Cache eviction iterates all entries on every write**
   
   The `setIpinfoPrivacyCache` cleanup loop (lines 113–117) walks the entire map on every write to remove expired entries. With up to 5 000 entries this is O(n) per write. A simpler approach: skip the expired-eviction loop entirely and rely only on the size-cap eviction — expired entries are harmless since `get` re-checks `expiresAt` before returning. The size cap already bounds memory; stale entries will be evicted as new ones arrive.
   
   How can I resolve this? If you propose a fix, please make it concise.

3. web/src/app/api/v1/freebuff/session/_handlers.ts, line 216-253 (link)

   Full IPinfo check runs on every GET poll

   getFreebuffSession calls countryBlockedResponse, which does a complete lookupIpinfoPrivacy round-trip (cached for 30 min) on every poll. The CLI polls GET every few seconds while in the queue. Once the 30-minute in-process cache expires — or on a fresh pod with an empty cache — every poll fires a live IPinfo request, and since the PR is intentionally fail-closed, a brief IPinfo outage will block the user with country_blocked even while they're actively queued.

   The stored country_checked_at and country_code on the row are never consulted here. Consider skipping the re-check on GET when the row already has a recent country_checked_at (e.g. within the same cache TTL window), or at minimum document that GET is intentionally fail-closed so the operational expectation is explicit.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: web/src/app/api/v1/freebuff/session/_handlers.ts
   Line: 216-253
   
   Comment:
   **Full IPinfo check runs on every GET poll**
   
   `getFreebuffSession` calls `countryBlockedResponse`, which does a complete `lookupIpinfoPrivacy` round-trip (cached for 30 min) on every poll. The CLI polls GET every few seconds while in the queue. Once the 30-minute in-process cache expires — or on a fresh pod with an empty cache — every poll fires a live IPinfo request, and since the PR is intentionally fail-closed, a brief IPinfo outage will block the user with `country_blocked` even while they're actively queued.
   
   The stored `country_checked_at` and `country_code` on the row are never consulted here. Consider skipping the re-check on GET when the row already has a recent `country_checked_at` (e.g. within the same cache TTL window), or at minimum document that GET is intentionally fail-closed so the operational expectation is explicit.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: web/src/server/free-mode-country.ts
Line: 185-202

Comment:
**`getIpPrivacy` wrapper can be inlined**

This function only (a) selects between the injected stub and the real implementation, and (b) wraps the call in a try-catch. Both concerns can live directly in `getFreeModeCountryAccess`, removing an indirection layer that makes the control flow harder to follow. The function and its `options` coupling can then be removed entirely, keeping the privacy-lookup path in one place.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: web/src/server/free-mode-country.ts
Line: 108-129

Comment:
**Cache eviction iterates all entries on every write**

The `setIpinfoPrivacyCache` cleanup loop (lines 113–117) walks the entire map on every write to remove expired entries. With up to 5 000 entries this is O(n) per write. A simpler approach: skip the expired-eviction loop entirely and rely only on the size-cap eviction — expired entries are harmless since `get` re-checks `expiresAt` before returning. The size cap already bounds memory; stale entries will be evicted as new ones arrive.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: web/src/app/api/v1/freebuff/session/_handlers.ts
Line: 57-80

Comment:
**`countryBlockedResponse` return type union is redundant**

Both union branches have identical shapes — the only difference is whether `response` is `NextResponse` or `null`. The discriminated union adds no real safety since callers check `if (blocked)` on the extracted `response` anyway. Simplify to a flat type:

```typescript
async function countryBlockedResponse(
  req: NextRequest,
  deps: FreebuffSessionDeps,
): Promise<{ response: NextResponse | null; countryAccess: FreeModeCountryAccess }> {
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: web/src/app/api/v1/freebuff/session/_handlers.ts
Line: 216-253

Comment:
**Full IPinfo check runs on every GET poll**

`getFreebuffSession` calls `countryBlockedResponse`, which does a complete `lookupIpinfoPrivacy` round-trip (cached for 30 min) on every poll. The CLI polls GET every few seconds while in the queue. Once the 30-minute in-process cache expires — or on a fresh pod with an empty cache — every poll fires a live IPinfo request, and since the PR is intentionally fail-closed, a brief IPinfo outage will block the user with `country_blocked` even while they're actively queued.

The stored `country_checked_at` and `country_code` on the row are never consulted here. Consider skipping the re-check on GET when the row already has a recent `country_checked_at` (e.g. within the same cache TTL window), or at minimum document that GET is intentionally fail-closed so the operational expectation is explicit.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Harden freebuff country gating" | Re-trigger Greptile}

[greptile-apps]

countryBlockedResponse return type union is redundant

Both union branches have identical shapes — the only difference is whether response is NextResponse or null. The discriminated union adds no real safety since callers check if (blocked) on the extracted response anyway. Simplify to a flat type:

async function countryBlockedResponse(
  req: NextRequest,
  deps: FreebuffSessionDeps,
): Promise<{ response: NextResponse | null; countryAccess: FreeModeCountryAccess }> {

Prompt To Fix With AI

This is a comment left during a code review.
Path: web/src/app/api/v1/freebuff/session/_handlers.ts
Line: 57-80

Comment:
**`countryBlockedResponse` return type union is redundant**

Both union branches have identical shapes — the only difference is whether `response` is `NextResponse` or `null`. The discriminated union adds no real safety since callers check `if (blocked)` on the extracted `response` anyway. Simplify to a flat type:

```typescript
async function countryBlockedResponse(
  req: NextRequest,
  deps: FreebuffSessionDeps,
): Promise<{ response: NextResponse | null; countryAccess: FreeModeCountryAccess }> {
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!