From 69e2279def6e174fc38ede28bbfdbf1a89d95dc7 Mon Sep 17 00:00:00 2001 From: AWCMS-Micro Security Date: Sat, 25 Jul 2026 20:45:17 +0700 Subject: [PATCH] fix(cache): invalidasi edge cache tidak pernah benar-benar terjadi (#359) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Klien purge memakai metode HTTP kustom `BAN` — idiom Varnish yang lazim, dan yang saya kirim di #353 tanpa pernah menjalankannya lewat jalur aplikasi sungguhan. `fetch` milik Bun DIAM-DIAM MENULIS ULANG metode yang tidak dikenal menjadi `GET`. Diverifikasi pada Bun 1.3.14 terhadap Varnish sungguhan: permintaan tiba sebagai `ReqMethod GET`, tidak pernah menyentuh cabang ban di VCL, dilayani sebagai halaman biasa, dan membalas 200. Karena 200 itu, klien melaporkan purge BERHASIL padahal cache tidak pernah tersentuh — invalidasi yang sepenuhnya mati namun tampak sehat, baik dari kode maupun dari `bun run edge-cache:health` (yang memakai metode kustom yang sama). Dua perubahan menutup kelas kegagalan ini, bukan hanya gejalanya: 1. Transport menjadi `POST /__awcms-edge-cache/ban`. Tidak lagi bergantung pada metode kustom yang harus selamat melewati setiap klien HTTP di rantai — sesuatu yang lapisan ini tidak bisa verifikasi saat runtime. 2. Respons ban membawa penanda `X-Edge-Cache-Ban: ok` yang WAJIB ada. Sebuah 200 tanpa penanda kini dilaporkan GAGAL, bukan sukses: artinya yang menjawab bukan handler ban cache, melainkan sesuatu yang lain (biasanya aplikasi itu sendiri). Inilah yang membuat "sukses palsu" tidak mungkin terulang. Diverifikasi pada Varnish sungguhan: MISS -> HIT -> POST ban (200 + penanda) -> MISS; GET ke path ban 405; POST tanpa token 403. Unit test tidak bisa menangkap ini karena mereka men-stub `fetch`; dua test baru menutupnya dari sisi kontrak — satu menegaskan permintaan benar-benar POST ke path yang dituju, satu menegaskan 200 tanpa penanda dibaca sebagai gagal. Refs #359 Co-Authored-By: Claude Opus 5 (1M context) --- ...e-cache-invalidation-was-a-silent-no-op.md | 27 +++++++++ deploy/varnish/default.vcl | 28 +++++++++- docs/awcms-micro/edge-cache-varnish.md | 54 +++++++++++------- scripts/edge-cache-health.ts | 24 +++++--- src/lib/cache/edge-cache-purge.ts | 37 +++++++++++- src/lib/config/registry.ts | 2 +- tests/unit/edge-cache-invalidation.test.ts | 17 +++++- tests/unit/edge-cache-pressure.test.ts | 56 +++++++++++++++++++ 8 files changed, 210 insertions(+), 35 deletions(-) create mode 100644 .changeset/edge-cache-invalidation-was-a-silent-no-op.md diff --git a/.changeset/edge-cache-invalidation-was-a-silent-no-op.md b/.changeset/edge-cache-invalidation-was-a-silent-no-op.md new file mode 100644 index 00000000..91881377 --- /dev/null +++ b/.changeset/edge-cache-invalidation-was-a-silent-no-op.md @@ -0,0 +1,27 @@ +--- +"awcms-micro": patch +--- + +Perbaiki invalidasi edge cache yang **tidak pernah benar-benar terjadi** +(Issue #359). + +Klien purge memakai metode HTTP kustom `BAN`, idiom Varnish yang lazim. +Tetapi `fetch` milik Bun **diam-diam menulis ulang metode yang tidak dikenal +menjadi `GET`** (diverifikasi pada Bun 1.3.14 terhadap Varnish sungguhan: +permintaan tiba sebagai `ReqMethod GET`, dilayani sebagai halaman biasa, dan +membalas 200). Karena 200 itu, klien melaporkan purge **berhasil** padahal +cache tidak pernah tersentuh — invalidasi yang sepenuhnya mati namun tampak +sehat, baik dari kode maupun dari `bun run edge-cache:health`. + +Dua perubahan menutup kelas kegagalan itu: + +- transport menjadi `POST /__awcms-edge-cache/ban` sehingga tidak lagi + bergantung pada metode kustom yang harus selamat melewati setiap klien + HTTP di rantai; +- respons ban membawa penanda `X-Edge-Cache-Ban: ok` yang **wajib** ada. + Sebuah 200 tanpa penanda kini dilaporkan **gagal**, bukan sukses — artinya + yang menjawab bukan handler ban cache. + +Ditemukan dengan menjalankan publikasi sungguhan pada instance staging dan +mendapati cache tetap `HIT`; unit test tidak bisa menangkapnya karena +mereka men-stub `fetch`. diff --git a/deploy/varnish/default.vcl b/deploy/varnish/default.vcl index 693d5ad0..04180efd 100644 --- a/deploy/varnish/default.vcl +++ b/deploy/varnish/default.vcl @@ -44,7 +44,21 @@ acl purge_network { } sub vcl_recv { - if (req.method == "BAN") { + # Invalidation endpoint. + # + # Deliberately a POST to a reserved PATH rather than a custom `BAN` + # method, which is the conventional Varnish idiom: Bun's `fetch` + # silently rewrites an unknown method to `GET` (verified on Bun 1.3.14 + # against this very service — the request arrived as `ReqMethod GET`, + # was served as an ordinary page, and returned 200, so the caller could + # not tell an invalidation had never happened). Relying on a custom verb + # surviving every HTTP client in the chain is not something this layer + # can verify at runtime, so it does not depend on it. + if (req.url == "/__awcms-edge-cache/ban") { + if (req.method != "POST") { + return (synth(405, "Method Not Allowed")); + } + if (client.ip !~ purge_network) { return (synth(403, "Forbidden")); } @@ -162,6 +176,18 @@ sub vcl_backend_response { return (deliver); } +sub vcl_synth { + # Marker the caller checks. Without it, ANY 200 reaching the client — + # including an ordinary page served because the request never matched + # the branch above — would read as a successful invalidation. That is + # exactly the failure this endpoint was rebuilt to make impossible. + if (resp.reason == "Banned") { + set resp.http.X-Edge-Cache-Ban = "ok"; + } + + return (deliver); +} + sub vcl_deliver { unset resp.http.X-Ban-Host; unset resp.http.X-Ban-Url; diff --git a/docs/awcms-micro/edge-cache-varnish.md b/docs/awcms-micro/edge-cache-varnish.md index 43fe83e5..242c18c1 100644 --- a/docs/awcms-micro/edge-cache-varnish.md +++ b/docs/awcms-micro/edge-cache-varnish.md @@ -137,15 +137,15 @@ database sedang tertekan. **Varnish 7.7.3 sungguhan** dengan backend tiruan, dan setiap aturannya diperiksa satu per satu: -| Yang diuji | Hasil | -| ------------------------------- | ----------------------------------------------------------- | -| Halaman publik diminta dua kali | MISS lalu **HIT**, backend hanya dipukul sekali | -| Permintaan dengan cookie sesi | Selalu MISS — backend dipukul setiap kali (bypass benar) | -| Permintaan dengan cookie locale | MISS lalu **HIT** — varian per-locale, bukan per-pengunjung | -| `/admin` dua kali | Tidak pernah HIT | -| Response ber-`Set-Cookie` | Tidak pernah disimpan | -| BAN tanpa token / token salah | **403** | -| BAN dengan token benar | 200, dan permintaan berikutnya kembali MISS | +| Yang diuji | Hasil | +| ------------------------------------ | ----------------------------------------------------------- | +| Halaman publik diminta dua kali | MISS lalu **HIT**, backend hanya dipukul sekali | +| Permintaan dengan cookie sesi | Selalu MISS — backend dipukul setiap kali (bypass benar) | +| Permintaan dengan cookie locale | MISS lalu **HIT** — varian per-locale, bukan per-pengunjung | +| `/admin` dua kali | Tidak pernah HIT | +| Response ber-`Set-Cookie` | Tidak pernah disimpan | +| Invalidasi tanpa token / token salah | **403** | +| Invalidasi token benar | 200 + `X-Edge-Cache-Ban: ok`, permintaan berikutnya MISS | Pengujian itu menemukan satu cacat nyata: `Surrogate-Control` masih terkirim ke klien pada response yang **tidak** di-cache, karena dulu hanya @@ -173,15 +173,15 @@ satu siklus deploy penuh (19 HIT dari 20, `db_xact` 1–4). Aturan keamanannya diverifikasi pada instance yang sama, bukan hanya di lab: -| Yang diuji | Hasil | -| ----------------------------- | ------------------------------------------- | -| Halaman publik, 2× | HIT — backend tidak dipukul lagi | -| Permintaan dengan cookie sesi | `bypass`, selalu MISS | -| `/admin`, 2× | Tidak pernah HIT | -| `/api/v1/health`, 2× | `bypass`, selalu MISS | -| BAN tanpa token / token salah | **403** | -| BAN token benar | 200, dan permintaan berikutnya kembali MISS | -| `Surrogate-Control` ke klien | Tidak pernah muncul, pada rute mana pun | +| Yang diuji | Hasil | +| ------------------------------------ | ------------------------------------------- | +| Halaman publik, 2× | HIT — backend tidak dipukul lagi | +| Permintaan dengan cookie sesi | `bypass`, selalu MISS | +| `/admin`, 2× | Tidak pernah HIT | +| `/api/v1/health`, 2× | `bypass`, selalu MISS | +| Invalidasi tanpa token / token salah | **403** | +| Invalidasi token benar | 200, dan permintaan berikutnya kembali MISS | +| `Surrogate-Control` ke klien | Tidak pernah muncul, pada rute mana pun | ### Bila Cloudflare ikut berada di depan (record ter-proxy) @@ -222,8 +222,22 @@ tidak pernah bisa menjangkau di luar hostname yang disebutnya. Pola path dibatasi himpunan karakter sebelum dikirim, karena pola itu ikut membentuk ekspresi ban di dalam cache. -Cache menerima BAN hanya dari jaringan privat **dan** dengan token yang -cocok. Token kosong **mematikan** invalidasi, bukan membukanya. +Cache menerima invalidasi hanya dari jaringan privat **dan** dengan token +yang cocok. Token kosong **mematikan** invalidasi, bukan membukanya. + +**Transportnya `POST /__awcms-edge-cache/ban`, bukan metode HTTP `BAN`** — +dan itu bukan selera. Idiom Varnish yang lazim memakai metode kustom `BAN`, +tetapi `fetch` milik Bun **diam-diam menulis ulang metode yang tidak dikenal +menjadi `GET`** (diverifikasi pada Bun 1.3.14 terhadap Varnish sungguhan: +permintaan tiba sebagai `ReqMethod GET`, dilayani sebagai halaman biasa, dan +membalas 200). Karena 200 itu, klien mengira purge berhasil padahal tidak +pernah terjadi — invalidasi yang sepenuhnya mati namun tampak sehat. + +Dua hal menutup kelas kegagalan itu: transport tidak lagi bergantung pada +metode kustom, dan respons ban membawa penanda `X-Edge-Cache-Ban: ok` yang +**wajib** ada. Sebuah 200 tanpa penanda itu kini dilaporkan sebagai +**gagal**, bukan sukses — karena artinya yang menjawab bukan handler ban +cache, melainkan sesuatu yang lain (biasanya aplikasi itu sendiri). ### Purge otomatis saat publikasi berubah (Issue #359) diff --git a/scripts/edge-cache-health.ts b/scripts/edge-cache-health.ts index bc2bc524..dd5761d3 100644 --- a/scripts/edge-cache-health.ts +++ b/scripts/edge-cache-health.ts @@ -58,14 +58,22 @@ async function probePurgeEndpoint(): Promise { // Deliberately WITHOUT the token: a correctly configured cache must // answer 4xx here. A 2xx would mean anyone on the network can flush // the cache, which is worth failing this check over. - const response = await fetch(config.purgeUrl, { - method: "BAN", - headers: { - "X-Ban-Host": "edge-cache-health.invalid", - "X-Ban-Path": "^/" - }, - signal: AbortSignal.timeout(3_000) - }); + // + // POST to the reserved path, matching `edge-cache-purge.ts` — a custom + // HTTP method does not survive Bun's `fetch` (it is rewritten to + // `GET`), which is what let a completely non-functional invalidation + // look healthy before. + const response = await fetch( + new URL("/__awcms-edge-cache/ban", config.purgeUrl), + { + method: "POST", + headers: { + "X-Ban-Host": "edge-cache-health.invalid", + "X-Ban-Path": "^/" + }, + signal: AbortSignal.timeout(3_000) + } + ); if (response.ok) { return { diff --git a/src/lib/cache/edge-cache-purge.ts b/src/lib/cache/edge-cache-purge.ts index 835ca289..b6a0a150 100644 --- a/src/lib/cache/edge-cache-purge.ts +++ b/src/lib/cache/edge-cache-purge.ts @@ -19,6 +19,22 @@ import { loadEdgeCacheConfig, type EdgeCacheConfig } from "./edge-cache-config"; const PURGE_TIMEOUT_MS = 2_000; +/** + * Reserved path the cache intercepts, and the marker header its synthetic + * response carries. + * + * A POST to a reserved path, NOT a custom `BAN` method: Bun's `fetch` + * silently rewrites an unknown method to `GET` (verified on Bun 1.3.14 + * against a live Varnish — the request arrived as `GET`, was served as an + * ordinary page, and returned 200). The old code read that 200 as a + * successful purge, so invalidation was a silent no-op that reported + * success. Checking the marker below is what makes that class of failure + * impossible to repeat: a 200 from anything that is not the cache's own ban + * handler is now a failure, not a pass. + */ +const BAN_PATH = "/__awcms-edge-cache/ban"; +const BAN_MARKER_HEADER = "x-edge-cache-ban"; + /** Hostnames only — no scheme, port, path, or wildcard. */ const HOST_PATTERN = /^[A-Za-z0-9.-]{1,253}$/; @@ -76,8 +92,9 @@ export async function purgeEdgeCache( const pathPattern = request.pathPattern ?? "^/"; try { - const response = await fetch(config.purgeUrl, { - method: "BAN", + const endpoint = new URL(BAN_PATH, config.purgeUrl); + const response = await fetch(endpoint, { + method: "POST", headers: { "X-Ban-Host": request.host, "X-Ban-Path": pathPattern, @@ -86,6 +103,22 @@ export async function purgeEdgeCache( signal: AbortSignal.timeout(PURGE_TIMEOUT_MS) }); + if (response.ok && response.headers.get(BAN_MARKER_HEADER) !== "ok") { + recordCounter("edge_cache_purge_total", { outcome: "failed" }); + + // A 200 without the marker means something OTHER than the cache's ban + // handler answered — most likely the request reached the application + // itself. Reporting that as success is what hid a completely + // non-functional invalidation before. + log("warning", "edge_cache.purge.unmarked_response", { + moduleKey: "deployment", + host: request.host, + statusCode: response.status + }); + + return { status: "failed", reason: "unmarked_response" }; + } + if (!response.ok) { recordCounter("edge_cache_purge_total", { outcome: "failed" }); diff --git a/src/lib/config/registry.ts b/src/lib/config/registry.ts index 41be439e..ae5d2303 100644 --- a/src/lib/config/registry.ts +++ b/src/lib/config/registry.ts @@ -2383,7 +2383,7 @@ export const CONFIG_REGISTRY: readonly ConfigVarEntry[] = [ sensitivity: "secret", profiles: ALL_PROFILES, description: - "Shared secret the cache requires on a BAN request, alongside its own private-network ACL. SECRET — never in an issue, log, or screenshot. Empty on either side disables invalidation rather than accepting an unauthenticated purge." + "Shared secret the cache requires on an invalidation request (`POST /__awcms-edge-cache/ban`), alongside its own private-network ACL. SECRET — never in an issue, log, or screenshot. Empty on either side disables invalidation rather than accepting an unauthenticated purge." }, // --------------------------------------------------------------------- // Preflight tooling (Issue #293) diff --git a/tests/unit/edge-cache-invalidation.test.ts b/tests/unit/edge-cache-invalidation.test.ts index f80684c4..e2404345 100644 --- a/tests/unit/edge-cache-invalidation.test.ts +++ b/tests/unit/edge-cache-invalidation.test.ts @@ -105,7 +105,10 @@ describe("invalidatePublicCacheForTenant — configured", () => { path: headers.get("x-ban-path") ?? "" }); - return new Response(null, { status: 200 }); + return new Response(null, { + status: 200, + headers: { "X-Edge-Cache-Ban": "ok" } + }); }) as typeof fetch; const { sql } = createSqlSpy([ @@ -151,7 +154,12 @@ describe("invalidatePublicCacheForTenant — configured", () => { globalThis.fetch = (async () => { call += 1; - return new Response(null, { status: call === 1 ? 200 : 500 }); + return call === 1 + ? new Response(null, { + status: 200, + headers: { "X-Edge-Cache-Ban": "ok" } + }) + : new Response(null, { status: 500 }); }) as unknown as typeof fetch; const { sql } = createSqlSpy([ @@ -253,7 +261,10 @@ describe("withPublicCacheInvalidation", () => { test("returns the original response object untouched on success", async () => { globalThis.fetch = (async () => - new Response(null, { status: 200 })) as unknown as typeof fetch; + new Response(null, { + status: 200, + headers: { "X-Edge-Cache-Ban": "ok" } + })) as unknown as typeof fetch; const { sql } = createSqlSpy([{ hostname: "tenant.example.com" }]); const success = new Response(JSON.stringify({ ok: true }), { status: 200 }); diff --git a/tests/unit/edge-cache-pressure.test.ts b/tests/unit/edge-cache-pressure.test.ts index beb86f66..caa59de3 100644 --- a/tests/unit/edge-cache-pressure.test.ts +++ b/tests/unit/edge-cache-pressure.test.ts @@ -232,6 +232,62 @@ describe("purgeEdgeCache", () => { ).resolves.toEqual({ status: "skipped", reason: "invalid_request" }); }); + test("POSTs to the reserved ban path, never a custom HTTP method", async () => { + // Bun's fetch silently rewrites an unknown method (e.g. `BAN`) to + // `GET`, which made invalidation a silent no-op that still reported + // success. Verified against a live Varnish on Bun 1.3.14. + const seen: { method?: string; url?: string } = {}; + const originalFetch = globalThis.fetch; + + globalThis.fetch = (async (url: URL | string, init?: RequestInit) => { + seen.method = init?.method; + seen.url = String(url); + + return new Response(null, { + status: 200, + headers: { "X-Edge-Cache-Ban": "ok" } + }); + }) as unknown as typeof fetch; + + try { + const result = await purgeEdgeCache( + { host: "tenant.example.com" }, + CONFIG + ); + + expect(result).toEqual({ status: "purged" }); + expect(seen.method).toBe("POST"); + expect(seen.url).toBe( + "http://varnish.invalid:8080/__awcms-edge-cache/ban" + ); + } finally { + globalThis.fetch = originalFetch; + } + }); + + test("treats a 200 WITHOUT the cache's marker header as a failure", async () => { + // The exact regression: an ordinary page answered 200 because the + // request never reached the ban handler at all. Without this check that + // reads as a successful purge. + const originalFetch = globalThis.fetch; + + globalThis.fetch = (async () => + new Response("a normal page", { + status: 200 + })) as unknown as typeof fetch; + + try { + const result = await purgeEdgeCache( + { host: "tenant.example.com" }, + CONFIG + ); + + expect(result).toEqual({ status: "failed", reason: "unmarked_response" }); + } finally { + globalThis.fetch = originalFetch; + } + }); + test("fails open when the cache is unreachable", async () => { const result = await purgeEdgeCache({ host: "tenant.example.com" }, CONFIG);