fix(cms): unauthenticated single request kills the process via audit of a forged X-Lumi-Site

[khuepm]

Summary

A single unauthenticated HTTP request kills the CMS process. All it takes is an X-Lumi-Site header naming a site that does not exist, plus any API key.

Found while implementing #332 (PR #468) — the starter's own cross-tenant check was knocking over its CMS.

Reproduce

On a clean Docker instance (ghcr.io/khuepm/lumibase-cms), after bootstrap and creating a publishable key:

curl -s -o /dev/null -w "status=%{http_code}\n" \
  -H "authorization: Bearer $PUBLISHABLE_KEY" \
  -H "x-lumi-site: some-other-site" \
  http://localhost:1989/api/v1/items/posts
# status=401   ← correct

sleep 4
curl -s -o /dev/null -w "health=%{http_code}\n" http://localhost:1989/health
# health=000   ← the process is gone

Container log:

cause: PostgresError: insert or update on table "lumibase_audit_log"
  violates foreign key constraint "lumibase_audit_log_site_id_lumibase_sites_id_fk"
  detail: 'Key (site_id)=(some-other-site) is not present in table "lumibase_sites".'

Cause

Three things combine:

1. The tenant middleware validates shape, not existence. apps/cms/src/middleware/tenant.ts:26-43 runs isValidSiteId(headerSite) and then c.set('siteId', headerSite). some-other-site is shape-valid, so it becomes the request's site id.

2. The denial is audited under that same unverified id. apps/cms/src/middleware/auth.ts:94 — auditApiKeyUseDenied passes siteId: c.get('siteId'). lumibase_audit_log.site_id has a foreign key to lumibase_sites.id (packages/database/src/schema/security.ts:98), so the insert violates the constraint.

3. The audit worker rethrows inside a promise nobody awaits. AuditLogger.write does catch failures on the synchronous insert path (apps/cms/src/modules/audit/logger.ts:485-490), so that path is safe. But when a queue is configured the job goes through the batcher, and apps/cms/src/modules/audit/worker.ts:139-141 catches, logs, then throw err — while the flush is invoked as void this.scheduleFlush(), fire-and-forget. Nothing catches it ⇒ unhandled rejection ⇒ the process dies.

Why this matters

• No authentication needed. The key is rejected (401) — an attacker needs no valid key, just a key-shaped string and a header.
• One request. No flood required.
• Damage crosses tenants. The batcher groups several sites into one insert ... values(auditRows) (worker.ts:104-117), so a single bad row fails the whole batch — losing the audit records of the valid sites in it. That is lost security evidence, not just downtime.
• The comment at auth.ts:409 describes this audit as "best-effort; never throws". True of the synchronous path; not true of the queued one.

Suggested fix (not implemented)

Three layers, and all three are worth doing since each stops a different failure:

1. Do not audit under an unverified site id. For site_mismatch the real site is known (apiKey.siteId); write under that and keep the requested one in metadata (requestedSiteId is already there).
2. The worker must not throw from an unawaited promise. Drop the throw err at worker.ts:141, or make scheduleFlush catch. The batcher should also split or drop the offending row instead of failing the batch.
3. Consider verifying the site exists in withTenant. That is an extra query on a hot path, so it needs caching thought — but with (1) and (2) done it is defence in depth rather than a requirement.

Verified on

main @ 6a20441a, and on the published image ghcr.io/khuepm/lumibase-cms@sha256:3f125caa… (revision 683a0270).

A regression test should assert that after a request carrying a forged site header /health still answers 200 — and that audit rows for a valid site in the same batch survive.

The #332 starter currently keeps its cross-tenant probe behind LUMIBASE_VERIFY_CROSS_TENANT=1 so running cms:verify cannot knock over the user's CMS; re-enable it by default once this closes.

---

🇻🇳 Tóm tắt tiếng Việt

Vấn đề. Một request HTTP duy nhất, không cần xác thực, làm chết tiến trình CMS: chỉ cần gửi X-Lumi-Site trỏ tới một site không tồn tại kèm một API key bất kỳ. Key bị từ chối (401) rồi process chết (health = 000).

Nguyên nhân — ba chỗ cộng lại:

1. tenant.ts:26-43 chỉ kiểm tra hình dạng site id, không kiểm tra tồn tại, rồi đặt luôn vào context.
2. auth.ts:94 ghi audit bằng chính site id chưa xác minh đó; cột site_id có FK tới lumibase_sites (security.ts:98) nên insert vi phạm ràng buộc.
3. AuditLogger.write có bắt lỗi cho insert đồng bộ (logger.ts:485-490), nên đường đó an toàn. Crash đến từ đường queue: batcher bắt lỗi rồi throw err (worker.ts:139-141) trong flush gọi kiểu fire-and-forget (void this.scheduleFlush()) ⇒ unhandled rejection ⇒ chết process.

Vì sao nghiêm trọng: không cần xác thực; chỉ tốn một request; và batch gộp nhiều site trong một insert (worker.ts:104-117) nên một hàng hỏng làm mất luôn audit của các site hợp lệ — mất dấu vết bảo mật, không chỉ downtime. Comment ở auth.ts:409 ghi audit này "never throws" — đúng với đường đồng bộ, sai với đường queue.

Hướng sửa (gợi ý): (1) ghi audit dưới apiKey.siteId thật, để site client yêu cầu vào metadata; (2) worker không được throw từ promise không ai chờ, và nên bỏ/tách hàng hỏng thay vì để chết cả batch; (3) cân nhắc xác minh site tồn tại trong withTenant (phòng thủ chiều sâu, cần cân nhắc cache).

Kiểm chứng trên main @ 6a20441a và image đã phát hành sha256:3f125caa…. Test hồi quy nên khẳng định /health vẫn 200 sau request có header giả, và audit của site hợp lệ trong cùng batch không mất.