fix(p1-integration): validate.rs allows _schemas/fragments $ref + drop additionalProperties on fragment-composed atom schemas
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 <fragment>]`. 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) <noreply@anthropic.com>
This commit is contained in:
parent
30a07e22ca
commit
324ad5d53e
3 changed files with 36 additions and 6 deletions
|
|
@ -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<PathBuf> {
|
||||
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))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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" }
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@
|
|||
"allOf": [
|
||||
{ "$ref": "../../../../../_schemas/fragments/entity-base.json" }
|
||||
],
|
||||
"additionalProperties": false,
|
||||
"examples": [
|
||||
{ "id": 42, "created_at": 1714000000 }
|
||||
]
|
||||
|
|
|
|||
Loading…
Reference in a new issue