fix(mcp): keep loading api_key rows projected before the layered ACL

CLAUDE.md

@@ -135,6 +135,7 @@ The five kinds are the cross-plane taxonomy (cp-admin.yaml `kind`); this repo's
 - When you touch a model-keyed mechanism (a limit, a guard, an ACL, a config knob, usage/metric attribution, cache keying), answer in the doc comment: does it key on the **requested** entry, the **dispatched** target, or **both**, and what is the behavior for each of the six shapes.
 - 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.)
 - **`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

@@ -48,6 +48,14 @@ pub struct McpPolicy {
     /// exactly. An empty list allows nothing, which is how a policy blocks
     /// all MCP access; a policy that only means to subtract tools writes
     /// `["*"]` here and lists them under `deny`.
+    ///
+    /// The write path requires the field (the strict schema adds it to
+    /// `required`), so a layer never allows something by omission. The
+    /// runtime loader defaults it to empty instead of rejecting the row:
+    /// a document written before the layered shape would otherwise fail
+    /// to deserialize, and a skipped `api_key` row stops authenticating
+    /// altogether rather than merely losing MCP access.
+    #[serde(default)]
     pub allow: Vec<String>,
 
     /// Namespaced `<server>__<tool>` patterns subtracted from the effective
@@ -83,6 +91,10 @@ pub struct McpAccess {
     /// with the environment and team layers. Same single-`*` glob matching a
     /// policy's `allow` uses; an empty list leaves the key no MCP access,
     /// and `["*"]` narrows nothing (useful with `deny` alone).
+    ///
+    /// Required on the write path and defaulted by the runtime loader,
+    /// for the reason given on [`McpPolicy::allow`].
+    #[serde(default)]
     pub allow: Vec<String>,
 
     /// Namespaced `<server>__<tool>` patterns subtracted from this key's
@@ -144,9 +156,18 @@ mod tests {
     }
 
     #[test]
-    fn allow_is_required_on_both_layers() {
-        assert!(serde_json::from_str::<McpPolicy>(r#"{"scope":"env"}"#).is_err());
-        assert!(serde_json::from_str::<McpAccess>(r#"{"deny":["github__*"]}"#).is_err());
+    fn the_loader_defaults_a_missing_allow_to_empty() {
+        // A row written before the layered shape — e.g. the old
+        // `{"mode":"inherit"}` key block — must still deserialize. It
+        // resolves to a layer allowing nothing (fail-closed) rather than
+        // being skipped, which for an api_key would drop the whole key.
+        // The write path still rejects it; see
+        // `mcp_policy_requires_an_explicit_allow_side` in schema.rs.
+        let p: McpPolicy = serde_json::from_str(r#"{"scope":"env"}"#).unwrap();
+        assert!(p.allow.is_empty());
+
+        let a: McpAccess = serde_json::from_str(r#"{"mode":"inherit"}"#).unwrap();
+        assert!(a.allow.is_empty());
     }
 
     #[test]

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

@@ -92,15 +92,15 @@ fn closes_on_write(resource: &str) -> bool {
 pub fn resource_root_schema(resource: &str, strict: bool) -> Value {
     let mut schema = match resource {
         "model" => model_root_schema(strict),
-        "api_key" => apikey_root_schema(),
+        "api_key" => apikey_root_schema(strict),
         "provider_key" => provider_key_root_schema(),
         "guardrail" => guardrail_root_schema(),
         "guardrail_attachment" => guardrail_attachment_root_schema(),
         "cache_policy" => cache_policy_root_schema(),
         "observability_exporter" => observability_exporter_root_schema(),
         "rate_limit_policy" => rate_limit_policy_root_schema(),
         "mcp_server" => mcp_server_root_schema(),
-        "mcp_policy" => mcp_policy_root_schema(),
+        "mcp_policy" => mcp_policy_root_schema(strict),
         "a2a_agent" => a2a_agent_root_schema(),
         "oidc_provider" => oidc_provider_root_schema(),
         "claim_mapping" => claim_mapping_root_schema(),
@@ -468,8 +468,37 @@ pub fn model_root_schema(strict: bool) -> Value {
 /// `Option` representation so `team_id`/`user_id` keep accepting an explicit
 /// `null` (cp-api sends `null` to clear team/owner), matching the resource's
 /// wire contract.
-pub fn apikey_root_schema() -> Value {
-    struct_root_schema::<crate::models::ApiKey>(true)
+pub fn apikey_root_schema(strict: bool) -> Value {
+    let mut schema = struct_root_schema::<crate::models::ApiKey>(true);
+    if strict {
+        require_property(
+            schema
+                .pointer_mut("/definitions/McpAccess")
+                .expect("api_key schema defines McpAccess"),
+            "allow",
+        );
+    }
+    schema
+}
+
+/// Add `name` to a schema object's `required` list, creating the list
+/// when absent. Used for fields the WRITE path must see spelled out
+/// while the runtime loader defaults them — a stale row has to keep
+/// loading, but a new one must not acquire its meaning by omission.
+fn require_property(schema: &mut Value, name: &str) {
+    let obj = schema
+        .as_object_mut()
+        .expect("schema fragment is a JSON object");
+    match obj.get_mut("required").and_then(Value::as_array_mut) {
+        Some(list) => {
+            if !list.iter().any(|v| v.as_str() == Some(name)) {
+                list.push(json!(name));
+            }
+        }
+        None => {
+            obj.insert("required".to_string(), json!([name]));
+        }
+    }
 }
 
 /// Canonical JSON Schema for the `provider_key` resource, derived from the
@@ -791,8 +820,11 @@ pub fn mcp_auth_settings_root_schema() -> Value {
 /// cross-field invariant `schemars` cannot express: a `team`-scoped policy
 /// must name its team in `scope_ref` (otherwise the row could shadow the
 /// environment layer).
-pub fn mcp_policy_root_schema() -> Value {
+pub fn mcp_policy_root_schema(strict: bool) -> Value {
     let mut schema = struct_root_schema::<crate::models::McpPolicy>(true);
+    if strict {
+        require_property(&mut schema, "allow");
+    }
     schema
         .as_object_mut()
         .expect("mcp_policy root schema is a JSON object")
@@ -1980,6 +2012,27 @@ mod tests {
         validate_mcp_policy(&json!({"scope": "env", "allow": []})).unwrap();
     }
 
+    #[test]
+    fn the_lenient_schema_still_loads_a_pre_layer_row() {
+        // Documents projected before the layered shape carry a `mode`
+        // and no `allow`. The strict write path rejects them, but the
+        // runtime loader must still accept them: a rejected `api_key`
+        // row is skipped entirely, so the key would stop authenticating
+        // for ALL traffic rather than merely losing MCP access.
+        let key = json!({
+            "key_hash":"9df37f5e7cbc3c391d872742b5f286c242e733a09add9eeaa4d26a599bd90b20",
+            "allowed_models":[],
+            "allowed_tools":["github__*"],
+            "mcp_access": {"mode": "inherit"}
+        });
+        validate_apikey_lenient(&key).unwrap();
+        assert!(validate_apikey(&key).is_err());
+
+        let policy = json!({"scope": "env", "mode": "all"});
+        validate_mcp_policy_lenient(&policy).unwrap();
+        assert!(validate_mcp_policy(&policy).is_err());
+    }
+
     #[test]
     fn mcp_policy_requires_an_explicit_allow_side() {
         assert!(validate_mcp_policy(&json!({"scope": "env"})).is_err());

schemas/resources/api_key.schema.json

@@ -7,7 +7,8 @@
       "description": "The API key's own layer of the MCP tool ACL, the same `allow`/`deny` shape an MCP access policy carries. Present means the key constrains its grant; omitted means the key adds no constraint of its own and takes whatever the environment and team layers leave.",
       "properties": {
         "allow": {
-          "description": "Namespaced `<server>__<tool>` patterns this key allows, intersected with the environment and team layers. Same single-`*` glob matching a policy's `allow` uses; an empty list leaves the key no MCP access, and `[\"*\"]` narrows nothing (useful with `deny` alone).",
+          "default": [],
+          "description": "Namespaced `<server>__<tool>` patterns this key allows, intersected with the environment and team layers. Same single-`*` glob matching a policy's `allow` uses; an empty list leaves the key no MCP access, and `[\"*\"]` narrows nothing (useful with `deny` alone).\n\nRequired on the write path and defaulted by the runtime loader, for the reason given on [`McpPolicy::allow`].",
           "items": {
             "type": "string"
           },

schemas/resources/mcp_policy.schema.json

@@ -46,7 +46,8 @@
   },
   "properties": {
     "allow": {
-      "description": "Namespaced `<server>__<tool>` patterns this layer allows. Entries are matched as single-`*` globs: `\"*\"` allows every tool, `\"<server>__*\"` every tool on one server, and an entry without a `*` matches one tool exactly. An empty list allows nothing, which is how a policy blocks all MCP access; a policy that only means to subtract tools writes `[\"*\"]` here and lists them under `deny`.",
+      "default": [],
+      "description": "Namespaced `<server>__<tool>` patterns this layer allows. Entries are matched as single-`*` globs: `\"*\"` allows every tool, `\"<server>__*\"` every tool on one server, and an entry without a `*` matches one tool exactly. An empty list allows nothing, which is how a policy blocks all MCP access; a policy that only means to subtract tools writes `[\"*\"]` here and lists them under `deny`.\n\nThe write path requires the field (the strict schema adds it to `required`), so a layer never allows something by omission. The runtime loader defaults it to empty instead of rejecting the row: a document written before the layered shape would otherwise fail to deserialize, and a skipped `api_key` row stops authenticating altogether rather than merely losing MCP access.",
       "items": {
         "type": "string"
       },
@@ -82,8 +83,8 @@
     }
   },
   "required": [
-    "allow",
-    "scope"
+    "scope",
+    "allow"
   ],
   "title": "McpPolicy",
   "type": "object"