From 456847eaa1d9802ea749a7fe36253aa2fadf2c32 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 20 Jul 2026 21:58:42 +0200 Subject: [PATCH] fix(#2591): validate() rejects out-of-bounds/forward parent index (argus review) --- hive-c0re/src/job_queue/templates.rs | 17 ++++++++++++++--- hive-c0re/src/job_queue/tests.rs | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/hive-c0re/src/job_queue/templates.rs b/hive-c0re/src/job_queue/templates.rs index 460655c8..61852e1f 100644 --- a/hive-c0re/src/job_queue/templates.rs +++ b/hive-c0re/src/job_queue/templates.rs @@ -256,9 +256,9 @@ pub fn meta_update( // per-agent child DAGs. /// Validate a spec before it enters the queue: node ids are dense -/// (index = id), deps reference existing nodes, and the dep graph is -/// acyclic (petgraph `toposort`). Rejecting cycles here fixes the old -/// queue's documented "circular dep silently deadlocks forever" caveat. +/// (index = id), deps + parents reference existing *earlier* nodes, and the +/// dep graph is acyclic (petgraph `toposort`). Rejecting cycles here fixes the +/// old queue's documented "circular dep silently deadlocks forever" caveat. pub fn validate(spec: &DagSpec) -> Result<()> { if spec.nodes.is_empty() { bail!("dag spec {:?} has no nodes", spec.template); @@ -269,6 +269,17 @@ pub fn validate(spec: &DagSpec) -> Result<()> { .map(|i| graph.add_node(u32::try_from(i).unwrap_or(u32::MAX))) .collect(); for (i, node) in spec.nodes.iter().enumerate() { + // A `parent` must index an earlier node — `insert_group` resolves it to + // an already-inserted `NodeId`, so a forward/out-of-bounds parent would + // otherwise panic there. + if let Some(p) = node.parent + && usize::try_from(p).is_ok_and(|p| p >= i) + { + bail!( + "dag spec {:?} node {i} has invalid parent {p} (must be an earlier node)", + spec.template + ); + } for dep in &node.deps { let Some(&dep_idx) = usize::try_from(dep.on).ok().and_then(|i| idx.get(i)) else { bail!( diff --git a/hive-c0re/src/job_queue/tests.rs b/hive-c0re/src/job_queue/tests.rs index 33608bae..2d184803 100644 --- a/hive-c0re/src/job_queue/tests.rs +++ b/hive-c0re/src/job_queue/tests.rs @@ -147,6 +147,21 @@ fn unknown_dep_is_rejected_at_submit() { assert!(q.submit(spec).is_err()); } +#[test] +fn invalid_parent_is_rejected_at_submit() { + let q = JobQueue::new(1); + let mut spec = rebuild("agent-a", "bad parent"); + // A forward/out-of-bounds parent index must be refused at validate, not + // panic in `insert_group`. + spec.nodes = vec![NodeSpec { + agent: "agent-a".to_owned(), + kind: NodeKind::Reconcile, + deps: Vec::new(), + parent: Some(3), + }]; + assert!(q.submit(spec).is_err()); +} + // ---- dependency order within a DAG ---- #[test]