fix(mcp): consume the control plane's legacy mcp_access mode tombstone

CLAUDE.md

@@ -136,6 +136,7 @@ The five kinds are the cross-plane taxonomy (cp-admin.yaml `kind`); this repo's
 - The per-target invariant (`crates/aisix-proxy/AGENTS.md`: "a per-model gate binds each target") is written around `resolve_attempt_models` — the routing-group trunk. **Ensemble panel/judge (`ProxyModelCaller::call`, the streaming judge) and semantic targets (`semantic::resolve`) bypass that trunk**, so a gate wired only into the trunk is silently absent there (the 2026-08 audit found member IP allowlist, health consumption, and retries all missing on the semantic path for exactly this reason — #958). A new per-target gate must be wired into the sub-dispatch paths too, or explicitly deferred with a filed issue. Prefer routing every dispatch through one shared chokepoint so the family can't drift.
 - **Strict writes, lenient loads.** `model_one_of` has two variants: the **strict** schema (declarative resources file, the published `schemas/resources/model.schema.json`, every strict validator consumer) forbids a knob a kind never resolves — accepted-but-unread config is the #962 class; the **lenient** loader keeps the base XOR so stored rows written by an older build still load, with `Model::strip_kind_inapplicable` dropping the dead knob and reporting it as `inapplicable:<field>` through the partial-compat channel. The two lists MUST mirror each other exactly (strict-forbidden ⇔ lenient-stripped) — a field forbidden-but-not-stripped half-honors; stripped-but-not-forbidden vanishes on load while the write path accepts it. A knob is enforced exactly as written or rejected, never half-honored (#963).
 - **Never make a field of a projected resource required at the TYPE level.** Requiredness belongs in the strict schema (`require_property` in `models/schema.rs`), never in the struct: the loader validates leniently and then deserializes, and a row it cannot deserialize is **skipped entirely** (`aisix-etcd/src/loader.rs`). Skipping is survivable for a resource the request path treats as optional, but an `api_key` row that fails to load stops authenticating **every** kind of traffic, not just the feature whose field changed — a far worse outcome than the field defaulting. So a new non-`Option` field, or one that loses `#[serde(default)]`, silently turns every already-projected row into a dead one. Give it a serde default whose meaning is fail-closed, and add it to `required` in the strict schema so the write path still refuses to guess. The control plane must also re-emit the affected collection once (`ReprojectMcpAclOnce` is the pattern) — the stored shape changed, but nothing else re-projects a row whose *content* did not. (Lesson from #993: `allow` was required at the type level in #992, which made every key still projected as `mcp_access: {"mode": "inherit"}` unloadable.)
+- **Never change a projected field's shape or value domain in place — the previous release must keep loading the row.** The supported upgrade order is control plane first, then data planes, with an arbitrarily long window; a released DP is immutable, so whatever the new CP projects must still parse one release back. Unknown *fields* are tolerated by design (serde_ignored → partial-compat report), but a malformed *known* field — a reshape, a lost default, a new enum value — fails the row and the loader skips it whole (the blast radius of the rule above). A reshape therefore ships under a NEW field name (the old one is never reused), or as a **same-name dual-generation document** when the old and new keys don't collide: the CP emits one document valid for both generations, and this side carries a consumed-and-ignored tombstone for the old selector — `McpAccess::legacy_mode` is the template: `#[serde(default, rename = "...", skip_serializing)]` + `#[schemars(skip)]` so the strict write path still rejects it, with a comment naming the retirement condition. A new enum value in an existing field cannot be made safe DP-side at all (lenient parsing keeps enums closed — it row-kills every older DP), so the paired CP PR must gate it behind `dpCompatGate` until the fleet minimum reads it. Whenever new semantics are invisible to the old release, verify the old default direction there: fail-closed or no-op is required; if it is fail-open, the CP must project an old-shape tombstone at the most restrictive value.
 - **`ensemble` is an experimental surface.** Its known parity gaps — member `allowed_cidrs`/guardrail/cooldown/health consumption, Prometheus token+spend attribution, response caching, parent-level generic knobs — are deliberate TODOs under a single future design pass. Do NOT piecemeal-fix one gap ahead of that pass, and do NOT re-audit them as fresh findings. (The one exception is a marshal-family or shared-chokepoint change where covering ensemble is a one-line parallel edit, e.g. projecting an entry-level field the DP already enforces.)
 - Adding a NEW kind = sweeping every existing model-keyed mechanism against it (grep the kind predicates in `models/model.rs`; every hit re-answers the questions above).
 

crates/aisix-core/src/models/mcp_policy.rs

@@ -101,6 +101,23 @@ pub struct McpAccess {
     /// effective grant, using the same single-`*` glob matching as `allow`.
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub deny: Vec<String>,
+
+    /// Compatibility tombstone for the pre-0.10.0 `mode` selector. The
+    /// control plane projects `"mode": "deny"` alongside the layered
+    /// shape so a 0.9.x data plane — where `mode` is required and `deny`
+    /// means "no MCP tool access" — still loads the whole api_key row,
+    /// fail-closed, instead of skipping it (a skipped row stops the key
+    /// authenticating for EVERY kind of traffic). This generation
+    /// consumes and ignores the value; the field exists only so the
+    /// loader does not report the tombstone as partial compat on every
+    /// row. Any JSON shape is accepted so a malformed tombstone can
+    /// never kill the row. Hidden from the schemas — the strict write
+    /// path closes unknown fields, so resource authors cannot set it —
+    /// and never re-serialized. Retire together with the CP emission
+    /// once 0.9.x is out of the supported upgrade window.
+    #[serde(default, rename = "mode", skip_serializing)]
+    #[schemars(skip)]
+    pub legacy_mode: Option<serde_json::Value>,
 }
 
 impl Resource for McpPolicy {
@@ -225,6 +242,25 @@ mod tests {
         assert!(blocked.deny.is_empty());
     }
 
+    #[test]
+    fn mcp_access_consumes_the_legacy_mode_tombstone() {
+        // The CP projects `"mode": "deny"` next to the layered shape so a
+        // 0.9.x DP loads the row fail-closed. This generation reads its
+        // own half, tolerates any tombstone shape, and never re-emits it.
+        let a: McpAccess =
+            serde_json::from_str(r#"{"mode":"deny","allow":["github__*"],"deny":["x__y"]}"#)
+                .unwrap();
+        assert_eq!(a.allow, vec!["github__*"]);
+        assert_eq!(a.deny, vec!["x__y"]);
+        assert_eq!(a.legacy_mode, Some(serde_json::json!("deny")));
+
+        let malformed: McpAccess = serde_json::from_str(r#"{"allow":[],"mode":5}"#).unwrap();
+        assert_eq!(malformed.legacy_mode, Some(serde_json::json!(5)));
+
+        let v = serde_json::to_value(&a).unwrap();
+        assert!(v.get("mode").is_none());
+    }
+
     #[test]
     fn empty_deny_stays_off_the_wire() {
         let p: McpPolicy = serde_json::from_str(r#"{"scope":"env","allow":["*"]}"#).unwrap();

crates/aisix-core/src/models/schema.rs

@@ -1964,12 +1964,23 @@ mod tests {
 
     #[test]
     fn apikey_mcp_access_rejects_the_removed_mode_field() {
+        // Write path: `mode` is gone from the authored shape. Read path:
+        // the CP projects `"mode": "deny"` as a tombstone so 0.9.x DPs
+        // load the row fail-closed, and this generation's loader must
+        // keep accepting the hybrid document.
         let v = json!({
             "key_hash":"9df37f5e7cbc3c391d872742b5f286c242e733a09add9eeaa4d26a599bd90b20",
             "allowed_models":[],
             "mcp_access": {"mode": "inherit", "allow": ["*"]}
         });
         assert!(validate_apikey(&v).is_err());
+        validate_apikey_lenient(&v).unwrap();
+        validate_apikey_lenient(&json!({
+            "key_hash":"9df37f5e7cbc3c391d872742b5f286c242e733a09add9eeaa4d26a599bd90b20",
+            "allowed_models":[],
+            "mcp_access": {"mode": "deny", "allow": ["github__*"], "deny": ["x__y"]}
+        }))
+        .unwrap();
     }
 
     #[test]

crates/aisix-etcd/src/loader.rs

@@ -957,6 +957,31 @@ mod tests {
         );
     }
 
+    #[test]
+    fn api_key_legacy_mcp_access_mode_tombstone_is_not_partial_compat() {
+        // The CP projects `"mode": "deny"` inside `mcp_access` so a 0.9.x
+        // DP — where `mode` is required — loads the row fail-closed
+        // instead of skipping it. This generation consumes the tombstone
+        // (`McpAccess::legacy_mode`); it must not surface as a
+        // partial-compat field on every key that carries a block.
+        let entries = vec![raw(
+            "/aisix/api_keys/k-tombstone",
+            br#"{
+                "key_hash": "1460db1b6902f8b1fc2a40d9381a24d0fd22c3bc1b2c6f999c521da73776fbe0",
+                "allowed_models": ["m"],
+                "mcp_access": {"mode": "deny", "allow": ["github__*"], "deny": ["x__y"]}
+            }"#,
+            1,
+        )];
+        let (snap, stats) = build_snapshot("/aisix", &entries);
+        assert_eq!(stats.accepted, 1, "rejections: {:?}", stats.rejections);
+        assert!(stats.partially_compatible.is_empty());
+        let entry = snap.apikeys.get_by_id("k-tombstone").unwrap();
+        let access = entry.value.mcp_access.as_ref().unwrap();
+        assert_eq!(access.allow, vec!["github__*"]);
+        assert_eq!(access.deny, vec!["x__y"]);
+    }
+
     #[test]
     fn nested_unknown_field_reports_dotted_path() {
         let entries = vec![raw(

tests/e2e/src/cases/mcp-access-policy-e2e.test.ts

@@ -43,6 +43,7 @@ const KEY_T1_WIDE = "sk-mcp-t1-wide";
 const KEY_T2 = "sk-mcp-t2";
 const KEY_NARROW = "sk-mcp-t2-narrow";
 const KEY_BLOCKED = "sk-mcp-blocked";
+const KEY_TOMBSTONE = "sk-mcp-tombstone";
 
 const sha256 = (s: string) => createHash("sha256").update(s).digest("hex");
 
@@ -166,6 +167,7 @@ describe("mcp access policy e2e: env, team and key layers intersect", () => {
     [KEY_T2, ENV_GRANT],
     [KEY_NARROW, ["alpha__echo"]],
     [KEY_BLOCKED, []],
+    [KEY_TOMBSTONE, ["alpha__echo", "alpha__reverse"]],
   ];
 
   beforeAll(async () => {
@@ -231,6 +233,15 @@ describe("mcp access policy e2e: env, team and key layers intersect", () => {
       }),
     );
     await seed.createApiKey(keyDoc(KEY_BLOCKED, { mcp_access: { allow: [] } }));
+    // The exact hybrid document the control plane projects: the layered
+    // shape plus the `"mode": "deny"` tombstone that keeps a 0.9.x DP
+    // loading the row fail-closed. This generation must consume the
+    // tombstone — authenticate the key and enforce the allow/deny half.
+    await seed.createApiKey(
+      keyDoc(KEY_TOMBSTONE, {
+        mcp_access: { mode: "deny", allow: ["alpha__*"] },
+      }),
+    );
 
     // Probe EVERY key to its expected steady state: keys are written at
     // higher revisions than servers/policies, but each key's list also
@@ -250,6 +261,18 @@ describe("mcp access policy e2e: env, team and key layers intersect", () => {
     await beta?.close();
   });
 
+  test("a CP-projected legacy-mode tombstone is consumed, not enforced", async (ctx) => {
+    if (!etcdReachable || !app) return ctx.skip();
+
+    // `mode` would mean "no MCP access" one release back; here it must
+    // be inert — the key's own allow intersects the env layer as usual.
+    await expectList(KEY_TOMBSTONE, ["alpha__echo", "alpha__reverse"]);
+    const ok = await callTool(KEY_TOMBSTONE, "alpha__echo", "hi");
+    expect(ok).toEqual({ ok: true, text: "alpha:hi" });
+    const denied = await callTool(KEY_TOMBSTONE, "beta__echo", "hi");
+    expect(denied.ok).toBe(false);
+  });
+
   test("a key with no block of its own takes the env layer unchanged", async (ctx) => {
     if (!etcdReachable || !app) return ctx.skip();