diff --git a/docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md b/docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md index a78f2b8..1096388 100644 --- a/docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md +++ b/docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md @@ -497,11 +497,12 @@ aligned reads/writes over identical bytes. `Schema` error instead of a stack overflow. The plan compile runs *before* the layout build in `AlkTypeEngine::compile`, making it the engine's reference-graph gate — `LayoutBuilder`/`OffsetMap` - struct-recursion has no cycle guard and previously could recurse + struct-recursion had no cycle guard and previously could recurse unboundedly on such a document (a pre-existing untrusted-schema hazard, surfaced by the phase-7 `compile_rejects_cyclic_ref_graph` - test). Hardening the layout walkers' own recursion is a separate - cleanup, not required while the gate holds in the engine path. + test). (Resolved since: review #006 H2 added the shared + `walk_guard::check_ref_graph` guard at every standalone walker entry, + so the trust boundary no longer depends on the engine path.) - **Fingerprinting enables downstream uses.** Cross-run plan caching, `alkcall` schema handshake, and schema-version diagnostics all become possible without further API work — across `ReadPlan`, diff --git a/docs/architecture/validation.md b/docs/architecture/validation.md index 9ee5771..7102641 100644 --- a/docs/architecture/validation.md +++ b/docs/architecture/validation.md @@ -293,11 +293,12 @@ The expensive work happens once at schema load time: 2. Parse the root struct's `"endian"` annotation. 3. Compile the `ValidationPlan` — the value-domain constraint tree, with eager `$ref` resolution. Its compile walk rejects cyclic `$ref` - graphs with a clean `Schema` error *before* the layout computation: - the layout walkers' struct/union recursion has no cycle guard, so - ordering the plan compile first is what keeps a self-referential - (malicious or accidental) document a handleable error, not a stack - overflow. + graphs with a clean `Schema` error *before* the layout computation. + (The layout walkers now also guard themselves — each standalone + entry point runs the shared reference-graph check + (`walk_guard::check_ref_graph`, review #006 H2) — so the plan-first + ordering is belt-and-suspenders at engine compile, and the trust + boundary no longer depends on the call path.) 4. Compute the layout (`LayoutBuilder` for packed, `OffsetMap` for aligned). 5. If `json_schema` is `Some`, build the standard diff --git a/docs/plans/030-compiled-forms.md b/docs/plans/030-compiled-forms.md index 33ededd..e5adedd 100644 --- a/docs/plans/030-compiled-forms.md +++ b/docs/plans/030-compiled-forms.md @@ -735,9 +735,11 @@ are trait derives, `fingerprint` is an inherent method). > - **Engine:** `Arc` built at `compile` in both modes; > new accessor `validation_plan()`. `validate_bytes` walks the plan. > **Bonus:** `ValidationPlan::compile` runs before the layout build and -> serves as the engine's cyclic-`$ref` gate (the layout walkers have no -> cycle guard; a cyclic doc used to be a stack-overflow hazard there — -> now a clean `Schema` error, see ADR-012 Consequences). +> serves as the engine's cyclic-`$ref` gate. (As of the review #006 H2 +> fix, the layout walkers also carry their own guard — +> `walk_guard::check_ref_graph` runs at each standalone entry — so this +> ordering is now belt-and-suspenders rather than the only defense; the +> plan text below predates that fix.) > - **Remaining phase-7 bench work** (a `validate_bytes`-stream bench > in alktty) moves with the bench work into phase 8; a spot check > during development measured plan-validate at ~0.2 µs/call vs ~0.6 diff --git a/docs/reviews/006-implementation-review-030.md b/docs/reviews/006-implementation-review-030.md index a9b33cd..83c598d 100644 --- a/docs/reviews/006-implementation-review-030.md +++ b/docs/reviews/006-implementation-review-030.md @@ -1,5 +1,5 @@ --- -status: in-progress (H1, L1, H3, L5, L6 resolved 2026-09-02) +status: in-progress (H1, L1, H3, L5, L6, H2 resolved 2026-09-02) last_updated: 2026-09-02 reviewed_artifacts: - src/read_plan.rs @@ -95,11 +95,15 @@ sub-types, `fingerprint()` methods; `Hash` on `Endian`/ | Severity | Count | Status | |----------|------:|--------| -| High | 3 (H1, H2, H3) | H1, H3 resolved 2026-09-02 | +| High | 3 (H1, H2, H3) | H1, H2, H3 resolved 2026-09-02 | | Medium | 4 (M1, M2, M3, M4) | open | | Low | 6 (L1–L6) | L1, L5, L6 resolved 2026-09-02 | | Nit | 2 (N1, N2) | open | +All three Highs are now resolved. The remaining work is Medium/Low/Nit: +M1+M2 (same file, one session), M3 (trivial), M4 (per-fix posture), and +L2/L3/L4/N1/N2 opportunistic. + The three Highs are adversarial-input crashes (H1, H2) and a cross-consumer wire-layout convention gap (H3) — all three are the exact class of problem AGENTS.md §3 exists for, given that `alkcall` @@ -124,6 +128,12 @@ them became materially easier to hit with the 0.3.0 surface. written, L5 doc added, L6 roundtrip test added. 488 tests green, clippy `-D warnings` clean, wasm build green, `cargo doc --no-deps` zero warnings. +- **H2 (2026-09-02):** resolved — see the "Resolution (2026-09-02)" + block at the end of the H2 finding. Shared one-shot reference-graph + guard (`walk_guard::check_ref_graph`) added and run at the entry of + all three standalone walkers; full cyclic/deep-nesting/diamond test + family added. 501 tests green, clippy `-D warnings` clean, wasm build + green, `cargo doc --no-deps` zero warnings. --- @@ -316,6 +326,62 @@ calls require pre-validated documents — but that re-introduces the explicitly retired for `ReadPlan`. The shared guard is the right fix; the doc note is the minimum acceptable one. +**Resolution (2026-09-02) — shared one-shot graph guard, all three +walkers gated:** + +1. **New module `src/walk_guard.rs`** (private, `pub(crate)`) with + `check_ref_graph(&BastDoc)` — a single bounded walk over the + reachable reference graph that rejects depth > 128 + (`MAX_GRAPH_DEPTH`, matching the plan compilers' + `MAX_COMPILE_DEPTH`) and any `$ref` cycle, using the same + path-scoped cycle-set semantics the plan compilers use (diamond + refs compile; only genuine cycles trip). The walk covers every + carrier shape: inline structs recurse into fields, named defs are + entered via `resolve_ref`, union mappings and shared `fields` are + both checked, and arrays/records are seen through to their + element/value types (so a cycle behind an array-of-`$ref` hop is + caught — the probe-verified gap shape). Error text mirrors the plan + compilers' wording ("cyclic $ref through…", "compile depth + exceeded…") so downstream matching sees one shape. + +2. **All three standalone walkers run the guard at entry**, before any + recursion: `OffsetMap::compute`, `LayoutBuilder::new`, and + `materialize_aligned` (the last as defense-in-depth — a cyclic doc + can no longer produce an `OffsetMap`, but a mismatched + doc/map pairing must still fail with a clean `Schema` error, not + overflow). `AlkTypeEngine::compile`'s ValidationPlan gate is + unchanged and now redundant-but-harmless; its doc comment is + updated to say so. The full shared-guard recommendation was taken + (not the minimum doc note). + +3. **Behavioral side effect, net-positive:** `check_ref_graph` + resolves every reachable def eagerly (via `resolve_ref`), which + parses each def's full shape — so an invalid *non-root* def (e.g. a + `$defs` union that re-declares a shared field, or whose discriminator + field is missing/non-first) now surfaces at `LayoutBuilder::new` + instead of at `build()`. Four pre-existing H3 tests asserted the old + lazy-parse timing ("root parses, fails at build"); they were updated + to expect the same `Schema` error at `new()`. Earlier rejection of + the same malformed schemas — strictly better for untrusted input, + no accepted schema's behavior changed. + +4. **Test family added (12 tests):** in `walk_guard.rs` (self-cycle, + two-def cycle, diamond allowed, 201-def deep chain → clean depth + error), in `offset_map.rs` (cycle rejected at compute, two-def + cycle, cycle behind inline-struct + array hop, diamond still + computes), in `layout_builder.rs` (cycle rejected at `new`, two-def + cycle, diamond still builds), and in `materialize.rs` (cyclic doc + + unrelated map → guard fires before offset lookup, diamond still + materializes). No stack-overflow reproducers in the default suite — + the tests assert the clean-error half only (the overflow itself was + probe-verified SIGABRT in the review session; see the Methodology + warning about running it). + +Verified: 501 tests green (423 crate + 17 + 34 + 15 + 12 + 2 +pre-existing ignored), `cargo clippy --all-targets -- -D warnings` +clean, `cargo build --target wasm32-unknown-unknown --release` green, +`cargo doc --no-deps` zero warnings. + ### H3. Field-name-discriminator unions: builder, reader, and materializer disagree on layout and on discriminator position **Files**: `src/layout_builder.rs:485-527` (write side), @@ -861,9 +927,8 @@ Worth recording, because the findings shouldn't eclipse it: see the resolution block on the finding). 2. ~~**H3**~~ **resolved 2026-09-02** (with L5 + L6; see the resolution block on the finding). -3. **H2** — shared walk guard (or the minimum doc note if the full - guard is judged too invasive for 0.3.x), plus the cyclic-schema - tests for all three walkers. +3. ~~**H2** — shared walk guard~~ **resolved 2026-09-02** (see the + resolution block on the finding). 4. **M1 + M2** — same file, same test family; do together. 5. **M3** — trivial deletion (or the feature decision, if kept). 6. **M4** — ongoing: per-fix coverage extension as recommended above; diff --git a/src/engine.rs b/src/engine.rs index 21be5f9..ec7d106 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -144,11 +144,12 @@ impl AlkTypeEngine { }; let endian = struct_node.endian(); // The compiled value-domain constraint tree (ADR-012 §3) — built - // once here, walked per buffer by `validate_bytes`. It is also the - // engine's reference-graph gate: its compile walk rejects cyclic - // `$ref` graphs (with a clean `Schema` error) before the layout - // builders below, whose struct/union recursion has no cycle guard - // and would otherwise recurse unboundedly on such a document. + // once here, walked per buffer by `validate_bytes`. Its compile + // walk also rejects cyclic `$ref` graphs here, before the layout + // builders below (the standalone walkers now guard themselves + // via `walk_guard::check_ref_graph` — review #006 H2 — so this + // gate is engine-compile-time confirmation, not the only line of + // defense). let validation_plan = Arc::new(ValidationPlan::compile(&doc)?); let layout = match mode { LayoutMode::Packed => { diff --git a/src/layout_builder.rs b/src/layout_builder.rs index 1f3cb92..4218fd8 100644 --- a/src/layout_builder.rs +++ b/src/layout_builder.rs @@ -147,10 +147,14 @@ impl LayoutBuilder { /// /// # Errors /// - /// Returns [`AlkTypeError::Schema`] if the document is malformed or - /// the root type is not a struct. + /// Returns [`AlkTypeError::Schema`] if the document is malformed, the + /// root type is not a struct, or the reference graph nests deeper + /// than the walk depth cap (128) or contains a `$ref` cycle (review + /// #006 H2 — the build walk's struct/union recursion is unguarded, so + /// cyclic input must be rejected before the walk, not during it). pub fn new(bast_doc: &Value, root_name: &str) -> Result { let doc = BastDoc::new(bast_doc, root_name)?; + crate::walk_guard::check_ref_graph(&doc)?; let root_def = doc.root_def(); let struct_node = match root_def.kind() { BastDefKind::Struct(s) => s, @@ -587,6 +591,66 @@ mod tests { pairs.iter().map(|(k, v)| (k.to_string(), *v)).collect() } + // ----- H2: cyclic schemas rejected at new(), not stack overflow ------ + + #[test] + fn h2_cyclic_ref_rejected_at_new_not_stack_overflow() { + let root = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "me", "kind": { "$ref": "#/$defs/S" } } + ] } + } + }); + let err = LayoutBuilder::new(&root, "S").unwrap_err(); + match err { + AlkTypeError::Schema(reason) => { + assert!(reason.contains("cyclic"), "reason: {reason}"); + } + other => panic!("expected Schema error, got {other:?}"), + } + } + + #[test] + fn h2_two_def_cycle_rejected_at_new() { + let root = json!({ + "$defs": { + "A": { "kind": "struct", "fields": [ + { "name": "next", "kind": { "$ref": "#/$defs/B" } } + ] }, + "B": { "kind": "struct", "fields": [ + { "name": "back", "kind": { "$ref": "#/$defs/A" } } + ] } + } + }); + let err = LayoutBuilder::new(&root, "A").unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + } + + #[test] + fn h2_diamond_refs_still_build() { + let root = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "a", "kind": { "$ref": "#/$defs/Point" } }, + { "name": "b", "kind": { "$ref": "#/$defs/Point" } } + ] }, + "Point": { "kind": "struct", "fields": [ + { "name": "x", "kind": "uint16" } + ] } + } + }); + let layout = build(&root, "S", &var_sizes(&[])); + assert_eq!( + layout.get("a.x"), + Some(&FieldPosition { offset: 0, size: 2, kind: AlkTypeKind::Uint16 }) + ); + assert_eq!( + layout.get("b.x"), + Some(&FieldPosition { offset: 2, size: 2, kind: AlkTypeKind::Uint16 }) + ); + } + #[test] fn fixed_fields_packed_no_alignment_padding() { let root = json!({ @@ -1502,8 +1566,10 @@ mod tests { } } }); - let builder = LayoutBuilder::new(&root, "S").expect("root parses (union resolves lazily)"); - let err = builder.build(&HashMap::new()).unwrap_err(); + // H2: the graph check in `new()` eagerly parses every reachable + // def, so the union's invalid shape surfaces at `new()` — the + // same Schema error, earlier than the old lazy `build()` hit. + let err = LayoutBuilder::new(&root, "S").unwrap_err(); match err { AlkTypeError::Schema(reason) => { assert!(reason.contains("re-declares"), "reason: {reason}"); @@ -1566,8 +1632,7 @@ mod tests { } } }); - let builder = LayoutBuilder::new(&root, "S").expect("root parses (union resolves lazily)"); - let err = builder.build(&HashMap::new()).unwrap_err(); + let err = LayoutBuilder::new(&root, "S").unwrap_err(); match err { AlkTypeError::Schema(reason) => { assert!(reason.contains("more than once"), "reason: {reason}"); @@ -1600,8 +1665,7 @@ mod tests { } } }); - let builder = LayoutBuilder::new(&root, "S").expect("root parses (union resolves lazily)"); - let err = builder.build(&HashMap::new()).unwrap_err(); + let err = LayoutBuilder::new(&root, "S").unwrap_err(); match err { AlkTypeError::Schema(reason) => { assert!(reason.contains("no field"), "reason: {reason}"); @@ -1641,8 +1705,7 @@ mod tests { } } }); - let builder = LayoutBuilder::new(&root, "S").expect("root parses (union resolves lazily)"); - let err = builder.build(&HashMap::new()).unwrap_err(); + let err = LayoutBuilder::new(&root, "S").unwrap_err(); match err { AlkTypeError::Schema(reason) => { assert!(reason.contains("must be the first"), "reason: {reason}"); diff --git a/src/lib.rs b/src/lib.rs index 4a31c6a..3e32cb3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,6 +60,7 @@ pub mod sequential_reader; pub mod tunion; pub mod validation; pub mod validation_plan; +pub(crate) mod walk_guard; pub use bast_meta::BAST_META_SCHEMA; pub use bast::{ diff --git a/src/materialize.rs b/src/materialize.rs index a180f92..6bb5c89 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -459,6 +459,14 @@ fn materialize_plan_record( /// The root type must be a struct. `offset_map` must have been computed /// from the same BAST document. Endianness is read from the root struct's /// `endian` annotation (defaults to little-endian). +/// +/// # Errors +/// +/// Returns [`AlkTypeError::Schema`] if the root type is not a struct, or +/// if the reference graph nests deeper than the walk depth cap (128) or +/// contains a `$ref` cycle (review #006 H2 — the aligned walk's struct +/// recursion is unguarded, so cyclic input must be rejected before the +/// walk, not during it). pub fn materialize_aligned( doc: &BastDoc, buffer: &[u8], @@ -474,6 +482,7 @@ pub fn materialize_aligned( ))); } }; + crate::walk_guard::check_ref_graph(doc)?; let effective_endian = struct_node.endian(); materialize_struct_aligned(doc, struct_node, "", offset_map, effective_endian, buffer) } @@ -1434,6 +1443,61 @@ mod tests { materialize_aligned(doc, buffer, &offset_map) } + // ----- H2: cyclic schemas rejected, not stack overflow --------------- + + #[test] + fn h2_cyclic_ref_rejected_at_materialize_aligned_not_stack_overflow() { + // A cyclic doc can no longer produce an OffsetMap (compute runs + // the same guard), so the materializer's own guard is + // defense-in-depth for mismatched/misused inputs. Feed it a + // cyclic doc with an unrelated map — the graph check must fire + // before any offset lookup. + let cyclic = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "me", "kind": { "$ref": "#/$defs/S" } } + ] } + } + }); + let honest = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "x", "kind": "uint16" } + ] } + } + }); + let honest_doc = doc_from(&honest, "S"); + let map = crate::offset_map::OffsetMap::compute(&honest_doc).expect("map"); + let cyclic_doc = doc_from(&cyclic, "S"); + let err = materialize_aligned(&cyclic_doc, &[0u8; 4], &map).unwrap_err(); + match err { + AlkTypeError::Schema(reason) => { + assert!(reason.contains("cyclic"), "reason: {reason}"); + } + other => panic!("expected Schema error, got {other:?}"), + } + } + + #[test] + fn h2_diamond_refs_still_materialize_aligned() { + let root = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "a", "kind": { "$ref": "#/$defs/Point" } }, + { "name": "b", "kind": { "$ref": "#/$defs/Point" } } + ] }, + "Point": { "kind": "struct", "fields": [ + { "name": "x", "kind": "uint16" } + ] } + } + }); + let doc = doc_from(&root, "S"); + let buf = [1u8, 0, 2, 0]; + let v = materialize_aligned_strict(&doc, &buf).expect("materialize"); + assert_eq!(v["a"]["x"], json!(1)); + assert_eq!(v["b"]["x"], json!(2)); + } + #[test] fn materialize_aligned_fixed_size_array() { let root = json!({ diff --git a/src/offset_map.rs b/src/offset_map.rs index a97fa11..c7bd8ec 100644 --- a/src/offset_map.rs +++ b/src/offset_map.rs @@ -116,7 +116,11 @@ impl OffsetMap { /// /// # Errors /// - /// Returns [`AlkTypeError::Schema`] if the root type is not a struct. + /// Returns [`AlkTypeError::Schema`] if the root type is not a struct, + /// or if the reference graph nests deeper than the walk depth cap + /// (128) or contains a `$ref` cycle (review #006 H2 — the compute + /// walk's struct recursion is unguarded, so cyclic input must be + /// rejected before the walk, not during it). /// Returns [`AlkTypeError::Offset`] for unsupported type combinations /// encountered during the walk (e.g. unions, which are rejected in /// aligned mode per ADR-008). @@ -131,6 +135,7 @@ impl OffsetMap { ))); } }; + crate::walk_guard::check_ref_graph(doc)?; let mut ctx = ComputeCtx { doc, fields: Vec::new(), @@ -854,6 +859,85 @@ mod tests { assert!(reason.contains("ADR-008"), "reason: {reason}"); } + #[test] + fn h2_cyclic_ref_rejected_at_compute_not_stack_overflow() { + let root = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "me", "kind": { "$ref": "#/$defs/S" } } + ] } + } + }); + let doc = BastDoc::new(&root, "S").expect("cyclic doc parses"); + let err = OffsetMap::compute(&doc).unwrap_err(); + match err { + AlkTypeError::Schema(reason) => { + assert!(reason.contains("cyclic"), "reason: {reason}"); + } + other => panic!("expected Schema error, got {other:?}"), + } + } + + #[test] + fn h2_two_def_cycle_rejected_at_compute() { + let root = json!({ + "$defs": { + "A": { "kind": "struct", "fields": [ + { "name": "next", "kind": { "$ref": "#/$defs/B" } } + ] }, + "B": { "kind": "struct", "fields": [ + { "name": "back", "kind": { "$ref": "#/$defs/A" } } + ] } + } + }); + let doc = BastDoc::new(&root, "A").expect("cyclic doc parses"); + let err = OffsetMap::compute(&doc).unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + } + + #[test] + fn h2_cycle_through_ref_field_of_nested_struct_rejected() { + // The cycle sits behind an inline struct + array hop: the walk + // guard must see through composite carriers, not just top-level + // fields. + let root = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "inner", "kind": { "kind": "struct", "fields": [ + { "name": "items", "kind": { + "kind": "array", "element": { "$ref": "#/$defs/S" }, "count": 1 + } } + ] } } + ] } + } + }); + let doc = BastDoc::new(&root, "S").expect("cyclic doc parses"); + let err = OffsetMap::compute(&doc).unwrap_err(); + assert!( + matches!(err, AlkTypeError::Schema(_)), + "expected Schema error, got {err:?}" + ); + } + + #[test] + fn h2_diamond_refs_still_compute() { + let root = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "a", "kind": { "$ref": "#/$defs/Point" } }, + { "name": "b", "kind": { "$ref": "#/$defs/Point" } } + ] }, + "Point": { "kind": "struct", "fields": [ + { "name": "x", "kind": "uint16" } + ] } + } + }); + let doc = BastDoc::new(&root, "S").expect("doc"); + let m = OffsetMap::compute(&doc).expect("diamond refs compute"); + assert_eq!(m.get("a.x").map(|e| e.range), Some(ByteRange { start: 0, end: 2 })); + assert_eq!(m.get("b.x").map(|e| e.range), Some(ByteRange { start: 2, end: 4 })); + } + #[test] fn non_final_inline_string_rejected_in_aligned_mode() { let root = json!({ diff --git a/src/walk_guard.rs b/src/walk_guard.rs new file mode 100644 index 0000000..4feec76 --- /dev/null +++ b/src/walk_guard.rs @@ -0,0 +1,241 @@ +//! Shared reference-graph guard for the standalone schema walkers. +//! +//! [`check_ref_graph`] is the one-shot pre-walk `AlkTypeEngine::compile` +//! runs before any layout builder, and the one +//! [`crate::offset_map::OffsetMap::compute`], +//! [`crate::layout_builder::LayoutBuilder::new`], and +//! [`crate::materialize::materialize_aligned`] run at their own entry so +//! each is untrusted-input-safe when called without the engine. It +//! rejects reference graphs that nest deeper than [`MAX_GRAPH_DEPTH`] or +//! that contain a `$ref` cycle — the two shapes that would otherwise +//! overflow the walkers' unguarded struct/union recursion (review #006 +//! H2; AGENTS.md §3: a cyclic or adversarially deep schema must produce +//! a handleable error, not a stack overflow). +//! +//! The compiled forms keep their own inline guards (their compile walks +//! inline types too, so a graph check alone is not enough for them); the +//! error text intentionally mirrors theirs ("cyclic $ref through…", +//! "compile depth exceeded…") so downstream matching sees one shape. + +use crate::bast::{BastDefKind, BastDoc, BastStruct, BastType}; +use crate::error::AlkTypeError; +use std::collections::BTreeSet; + +/// Maximum `$ref`-graph depth accepted by [`check_ref_graph`]. Matches +/// the plan compilers' `MAX_COMPILE_DEPTH` (128) so every walker rejects +/// the same documents. +pub(crate) const MAX_GRAPH_DEPTH: usize = 128; + +pub(crate) fn depth_err(path: &str) -> AlkTypeError { + AlkTypeError::Schema(format!( + "schema walk: compile depth exceeded {MAX_GRAPH_DEPTH} at {path} \ + (cyclic $ref or adversarially deep nesting)" + )) +} + +pub(crate) fn cycle_err(name: &str, path: &str) -> AlkTypeError { + AlkTypeError::Schema(format!( + "schema walk: cyclic $ref through {name:?} at {path}" + )) +} + +/// Reject cyclic or over-deep `$ref` graphs before a recursive walker +/// sees the document. +/// +/// One walk over the reachable definitions: inline structs recurse into +/// their fields; named defs are entered with the path-scoped cycle set +/// (a def currently being expanded) and the depth counter. Diamond +/// references (two fields `$ref`-ing the same def, neither nested inside +/// the other) are allowed — `seen` is removed on exit, so only genuine +/// cycles trip it, the same semantics the plan compilers use. +pub(crate) fn check_ref_graph(doc: &BastDoc) -> Result<(), AlkTypeError> { + let root_def = doc.root_def(); + let mut seen = BTreeSet::new(); + let path = root_def.name().to_string(); + match root_def.kind() { + BastDefKind::Struct(s) => check_struct(doc, s, &path, 0, &mut seen), + BastDefKind::Union(u) => { + check_typeref_list( + doc, + u.mapping().iter().map(|(_, ty)| ty), + &path, + 0, + &mut seen, + )?; + check_field_list(doc, u.fields(), &path, 0, &mut seen) + } + BastDefKind::Enum(_) => Ok(()), + } +} + +fn check_struct( + doc: &BastDoc, + s: &BastStruct, + path: &str, + depth: usize, + seen: &mut BTreeSet, +) -> Result<(), AlkTypeError> { + if depth > MAX_GRAPH_DEPTH { + return Err(depth_err(path)); + } + check_field_list(doc, s.fields(), path, depth, seen) +} + +fn check_field_list( + doc: &BastDoc, + fields: &[crate::bast::BastField], + path: &str, + depth: usize, + seen: &mut BTreeSet, +) -> Result<(), AlkTypeError> { + for field in fields { + let field_path = format!("{path}.{}", field.name()); + check_typeref(doc, field.ty(), &field_path, depth, seen)?; + } + Ok(()) +} + +fn check_typeref( + doc: &BastDoc, + ty: &BastType, + path: &str, + depth: usize, + seen: &mut BTreeSet, +) -> Result<(), AlkTypeError> { + if depth > MAX_GRAPH_DEPTH { + return Err(depth_err(path)); + } + match ty { + BastType::Ref(r) => { + let name = r.name(); + if !seen.insert(name.to_string()) { + return Err(cycle_err(name, path)); + } + let def = doc.resolve_ref(r)?; + let def_path = format!("{path} -> {name}"); + let out = match def.kind() { + BastDefKind::Struct(s) => { + check_struct(doc, s, &def_path, depth + 1, seen) + } + BastDefKind::Union(u) => { + check_typeref_list(doc, u.mapping().iter().map(|(_, ty)| ty), &def_path, depth + 1, seen)?; + check_field_list(doc, u.fields(), &def_path, depth + 1, seen) + } + BastDefKind::Enum(_) => Ok(()), + }; + seen.remove(name); + out + } + BastType::Struct(s) => check_struct(doc, s, path, depth + 1, seen), + BastType::Union(u) => { + check_typeref_list(doc, u.mapping().iter().map(|(_, ty)| ty), path, depth + 1, seen)?; + check_field_list(doc, u.fields(), path, depth + 1, seen) + } + BastType::Array(a) => { + check_typeref(doc, a.element(), path, depth + 1, seen) + } + BastType::Record(r) => check_typeref(doc, r.values(), path, depth + 1, seen), + BastType::Primitive(_) | BastType::Enum(_) => Ok(()), + } +} + +fn check_typeref_list<'a, I>( + doc: &BastDoc, + tys: I, + path: &str, + depth: usize, + seen: &mut BTreeSet, +) -> Result<(), AlkTypeError> +where + I: IntoIterator, +{ + for ty in tys { + check_typeref(doc, ty, path, depth, seen)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn self_cycle_rejected() { + let root = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "me", "kind": { "$ref": "#/$defs/S" } } + ] } + } + }); + let doc = BastDoc::new(&root, "S").expect("cyclic doc parses"); + let err = check_ref_graph(&doc).unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + assert!(err.to_string().contains("cyclic"), "got {err:?}"); + } + + #[test] + fn two_def_cycle_rejected() { + let root = json!({ + "$defs": { + "A": { "kind": "struct", "fields": [ + { "name": "next", "kind": { "$ref": "#/$defs/B" } } + ] }, + "B": { "kind": "struct", "fields": [ + { "name": "back", "kind": { "$ref": "#/$defs/A" } } + ] } + } + }); + let doc = BastDoc::new(&root, "A").expect("cyclic doc parses"); + let err = check_ref_graph(&doc).unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + } + + #[test] + fn diamond_refs_allowed() { + let root = json!({ + "$defs": { + "S": { "kind": "struct", "fields": [ + { "name": "a", "kind": { "$ref": "#/$defs/Point" } }, + { "name": "b", "kind": { "$ref": "#/$defs/Point" } } + ] }, + "Point": { "kind": "struct", "fields": [ + { "name": "x", "kind": "uint16" } + ] } + } + }); + let doc = BastDoc::new(&root, "S").expect("doc"); + assert!(check_ref_graph(&doc).is_ok()); + } + + #[test] + fn deep_ref_chain_rejected_not_overflow() { + // 201 named defs chained by refs — depth 201 exceeds the cap of + // 128, but the check itself must complete (bounded stack, clean + // error). Note the chain uses distinct defs, so the cycle set + // never trips; only the depth cap stops it. + let mut defs = serde_json::Map::new(); + defs.insert( + "L200".to_string(), + json!({ "kind": "struct", "fields": [ { "name": "v", "kind": "uint8" } ] }), + ); + for i in (0..200).rev() { + defs.insert( + format!("L{i}"), + json!({ + "kind": "struct", + "fields": [ { "name": "next", "kind": { "$ref": format!("#/$defs/L{}", i + 1) } } ] + }), + ); + } + let root = json!({ "$defs": defs }); + let doc = BastDoc::new(&root, "L0").expect("deep doc parses (no cycle)"); + let err = check_ref_graph(&doc).unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + assert!( + err.to_string().contains("depth exceeded"), + "expected depth error, got {err:?}" + ); + } +} \ No newline at end of file