From 324ad5d53ef29615bd4a3eff3418739d31df5c7d Mon Sep 17 00:00:00 2001 From: Parfii-bot Date: Thu, 23 Apr 2026 04:53:26 +0800 Subject: [PATCH] fix(p1-integration): validate.rs allows _schemas/fragments $ref + drop additionalProperties on fragment-composed atom schemas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P1↔E1-audit-wave integration regressions caught by kei-runtime invoke_real_atom test. 1. LocalFileResolver (E1 SSRF hardening) rejected $ref to _schemas/fragments/ because the dir is OUTSIDE atom's schema parent. Fix: extend LocalFileResolver with `find_fragments_root()` — walks up from schema root looking for `_schemas/fragments/`. If found, allow $ref under EITHER schema root OR fragments root. Still rejects arbitrary filesystem $ref. 2. jsonschema injection of absolute $id now ALSO applied to fragment schemas loaded via LocalFileResolver.resolve(). Without this, a fragment declaring `$id: "_schemas/fragments/titled.json"` (relative) was resolved against parent schema's absolute $id, producing double prefix `_schemas/fragments/_schemas/fragments/titled.json`. 3. create-input.json + create-output.json had `additionalProperties: false` alongside `allOf: [$ref ]`. Draft-07 gotcha: additionalProperties at this level does NOT see properties inherited from $ref-ed fragment — caused 'title' unexpected rejection. Dropped the constraint on 2 fragment-composed schemas; kept on 4 standalone ones (search-input/output + add-dependency-input/output). Tests: kei-runtime 5/5 green; integration test passes. Co-Authored-By: Claude Opus 4.7 (1M context) --- _primitives/_rust/kei-runtime/src/validate.rs | 40 +++++++++++++++++-- .../kei-task/atoms/schemas/create-input.json | 1 - .../kei-task/atoms/schemas/create-output.json | 1 - 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/_primitives/_rust/kei-runtime/src/validate.rs b/_primitives/_rust/kei-runtime/src/validate.rs index 94eaf57..987ea1d 100644 --- a/_primitives/_rust/kei-runtime/src/validate.rs +++ b/_primitives/_rust/kei-runtime/src/validate.rs @@ -82,7 +82,9 @@ fn inject_absolute_id(schema: &mut Value, schema_path: &Path) { } /// `$ref` resolver that rejects every scheme except `file://`, AND rejects -/// any path that is not inside `root` (canonicalised). +/// any path that is not inside `root` OR the shared `_schemas/fragments/` dir +/// (canonicalised). The fragments dir is resolved by walking up from `root` +/// until a sibling `_schemas/fragments/` is found or we reach filesystem root. #[derive(Debug)] pub struct LocalFileResolver { root: PathBuf, @@ -92,6 +94,20 @@ impl LocalFileResolver { pub fn new(root: PathBuf) -> Self { Self { root } } + + /// Walk up from root to find workspace's `_schemas/fragments/`. Returns + /// canonicalised path if found. Allows atom schemas to $ref shared + /// fragments without opening the entire filesystem. + fn find_fragments_root(&self) -> Option { + let mut cur = self.root.as_path(); + loop { + let candidate = cur.join("_schemas").join("fragments"); + if let Ok(canon) = candidate.canonicalize() { + return Some(canon); + } + cur = cur.parent()?; + } + } } impl SchemaResolver for LocalFileResolver { @@ -117,17 +133,33 @@ impl SchemaResolver for LocalFileResolver { .root .canonicalize() .map_err(|e| anyhow::anyhow!("canonicalize root {}: {e}", self.root.display()))?; - if !canon.starts_with(&root_canon) { + let fragments_canon = self.find_fragments_root(); + let in_root = canon.starts_with(&root_canon); + let in_fragments = fragments_canon + .as_ref() + .map(|f| canon.starts_with(f)) + .unwrap_or(false); + if !in_root && !in_fragments { return Err(anyhow::anyhow!( - "file $ref escapes schema root: {} not under {}", + "file $ref escapes both schema root and fragments dir: {} not under {} or _schemas/fragments/", canon.display(), root_canon.display() )); } let f = std::fs::File::open(&canon) .map_err(|e| anyhow::anyhow!("open {}: {e}", canon.display()))?; - let doc: Value = serde_json::from_reader(f) + let mut doc: Value = serde_json::from_reader(f) .map_err(|e| anyhow::anyhow!("parse {}: {e}", canon.display()))?; + // Override any relative `$id` in the loaded fragment with its + // absolute file:// URL. Without this, a fragment declaring e.g. + // `$id: "_schemas/fragments/titled-content.json"` would be + // resolved relative to the parent schema's $id by jsonschema, + // producing a doubled prefix (`_schemas/fragments/_schemas/...`). + if let Some(obj) = doc.as_object_mut() { + if let Ok(abs_url) = Url::from_file_path(&canon) { + obj.insert("$id".to_string(), Value::String(abs_url.to_string())); + } + } Ok(Arc::new(doc)) } } diff --git a/_primitives/_rust/kei-task/atoms/schemas/create-input.json b/_primitives/_rust/kei-task/atoms/schemas/create-input.json index 883598e..d26ff52 100644 --- a/_primitives/_rust/kei-task/atoms/schemas/create-input.json +++ b/_primitives/_rust/kei-task/atoms/schemas/create-input.json @@ -15,7 +15,6 @@ }, "milestone_id": { "type": "integer", "minimum": 1 } }, - "additionalProperties": false, "examples": [ { "title": "Fix auth bug", "priority": "high" }, { "title": "Refactor router", "description": "Split monolith", "priority": "medium" } diff --git a/_primitives/_rust/kei-task/atoms/schemas/create-output.json b/_primitives/_rust/kei-task/atoms/schemas/create-output.json index c9a3f87..54253d9 100644 --- a/_primitives/_rust/kei-task/atoms/schemas/create-output.json +++ b/_primitives/_rust/kei-task/atoms/schemas/create-output.json @@ -6,7 +6,6 @@ "allOf": [ { "$ref": "../../../../../_schemas/fragments/entity-base.json" } ], - "additionalProperties": false, "examples": [ { "id": 42, "created_at": 1714000000 } ]